Remove NFSv4 inheritance flags in setfacl recursive mode
ClosedPublic
Actions

Authored by mhorne063_gmail.com on Apr 13 2018, 6:41 PM.

Details

Reviewers

emaste
trasz

Group Reviewers

manpages

Commits

rS339793: Don't set NFSv4 ACL inheritance flags on non-directories.

Summary

When modifying ACLs recursively, strip the inheritance flags from each of the ACL entries.

This code was extracted from the patch originally submitted to PR 155163

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

mhorne063_gmail.com created this revision.Apr 13 2018, 6:41 PM

mhorne063_gmail.com added a parent revision: D15060: Cleanup of setfacl.c.Apr 13 2018, 6:47 PM

No code changes, just updated from changes made to D15060

emaste added inline comments.Apr 27 2018, 5:28 PM

bin/setfacl/setfacl.c
129 ↗	(On Diff #41921)	This should probably have a comment to give more information about what is meant by "sanitize".
153–155 ↗	(On Diff #41921)	style(9) puts the operator at the end of the line when wrapping is necessary

emaste added a subscriber: trasz.Apr 27 2018, 6:21 PM

It has been quite a few years since I originally wrote this code. This might be able to carve out some time these next two weeks to refactor it. There are parts of this patch that feel a bit awkward and could likely be improved upon.

Part of me wonders if the inheritance sanitization code should live in libc with the rest of the ACL code. Doing so would allow other projects that deal with ACLs to take advantage of it.

Cleaned this up a bit. The logic is the same but should be a bit more obvious now.

Unless I missed something there was no reason to be calling acl_dup() like we did in the last iteration of the patch.

Also made it more obvious that we were ignoring return values by casting them to void

mhorne063_gmail.com marked 2 inline comments as done.Oct 11 2018, 4:45 AM

markj added a reviewer: trasz.Oct 26 2018, 3:23 PM

markj added a subscriber: markj.

markj added inline comments.Oct 26 2018, 3:58 PM

bin/setfacl/setfacl.c
143 ↗	(On Diff #49000)	The callers are already checking R_flag, this is redundant.
154 ↗	(On Diff #49000)	Don't we also need to call acl_set_flagset_np()? This call is just modifying a stack-local variable.

markj added inline comments.Oct 26 2018, 4:07 PM

bin/setfacl/setfacl.c
131 ↗	(On Diff #49000)	This should probably be documented in the man page.
149 ↗	(On Diff #49000)	acl_flagset will be left uninitialized if an error occurs. This may not be possible in practice, but we should handle it anyway.