syslogd: unlink() -> unlinkat()
ClosedPublic
Actions

Authored by jfree on Aug 8 2023, 4:34 PM.

Details

Reviewers

emaste
markj

Commits

rG5b44c24f8524: syslogd: unlink() -> unlinkat()

Summary

Save basename and dirname for use in unlinkat() when removing UNIX
sockets from fs.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

jfree created this revision.Aug 8 2023, 4:34 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 8 2023, 4:34 PM

jfree requested review of this revision.Aug 8 2023, 4:34 PM

jfree added a parent revision: D41378: syslogd: Fix prop_filter memory leak.

jfree added a child revision: D41380: syslogd: Do not open console descriptor.

Can you use our funlinkat() instead of keeping track of the directory descriptor?

In D41379#942527, @markj wrote:

Can you use our funlinkat() instead of keeping track of the directory descriptor?

Yes. not sure how I missed that.

In D41379#943056, @jfree wrote:

In D41379#942527, @markj wrote:

Can you use our funlinkat() instead of keeping track of the directory descriptor?

Yes. not sure how I missed that.

Actually, after reading about this, I don't think so. It seems like funlinkat() purely exists to ensure that you're unlinking an inode that matches the given path. I don't think it can remove a file without the directory descriptor.

markj accepted this revision.Aug 11 2023, 3:03 PM

This revision is now accepted and ready to land.Aug 11 2023, 3:03 PM

Update after rebase

This revision now requires review to proceed.Aug 12 2023, 3:48 AM

markj accepted this revision.Aug 14 2023, 2:25 PM

markj added inline comments.

usr.sbin/syslogd/syslogd.c
2320	I'm not convinced we need to do this cleanup (aside from the unlinkat()) immediately prior to exiting. There's a lot of other stuff that isn't cleaned up.

This revision is now accepted and ready to land.Aug 14 2023, 2:25 PM

jfree added inline comments.Aug 14 2023, 2:37 PM

usr.sbin/syslogd/syslogd.c
2320	I'm not convinced we need to do this cleanup (aside from the unlinkat()) immediately prior to exiting. There's a lot of other stuff that isn't cleaned up. A good portion of this program is inconsistent in this matter. I'll remove it, though.

Remove cleanup in die()

This revision now requires review to proceed.Aug 14 2023, 3:46 PM

emaste accepted this revision.Aug 14 2023, 5:40 PM

emaste added inline comments.

usr.sbin/syslogd/syslogd.c
3764	separate from all of this work I wonder if following the `xstrdup` pattern makes sense
3768	Are we typically/often going to have more than one `AF_LOCAL` endpoint open, and would they share a directory? i.e., is it worth trying to cache and share `dirfd`s?