Thread (77 messages) 77 messages, 6 authors, 2025-11-18

Re: [PATCH v3 34/50] selinuxfs: new helper for attaching files to tree

From: Al Viro <viro@zeniv.linux.org.uk>
Date: 2025-11-11 09:50:00
Also in: bpf, linux-efi, linux-fsdevel, linux-mm, linux-usb, ocfs2-devel, selinux

On Tue, Nov 11, 2025 at 07:53:18AM +0000, bot+bpf-ci@kernel.org wrote:
Can this leak the parent directory's reference count? The parent inode's
link count is incremented with inc_nlink(d_inode(dir)) before calling
sel_attach(). When sel_attach()->d_alloc_name() fails and returns NULL,
sel_attach() correctly cleans up the child inode with iput() and returns
ERR_PTR(-ENOMEM). However, the parent directory's link count has already
been incremented and is never decremented on this error path.

In the original code, the parent link count increment happened after
d_add() succeeded, ensuring it only occurred when the full operation
completed successfully.
All callers of sel_make_dir() proceed to remove the parent in case of
failure.  All directories are created either at mount time or at
policy reload afterwards.  A failure in the former will have
sel_fill_super() return an error, with the entire filesystem instance
being torn apart by the cleanup path in its caller (get_tree_single()).
No directories survive that.  A failure in the latter (in something
called from sel_make_policy_nodes()) will be taken care of by the
call of simple_recursive_removal() in the end of sel_make_policy_nodes() -
there we
	1.  create a temporary directory ("/.swapover").  We do *NOT*
use sel_make_dir() for that - see sel_make_swapover_dir().  If that has
failed, we return an error.
	2.  create and populate two subtrees in it ("booleans" and "classes").
That's the step where we would create subdirectories and that's where
sel_make_dir() failures might occur.
	3.  if the subtree creation had been successful, swap "/.swapover/booleans"
with "/booleans" and "/.swapover/classes" with "/classes" respectively.
	4.  recursively remove "/.swapover", along with anything that might
be in it.  In case of success that would be the old "/classes" and "/booleans"
that got replaced, in case of failure - whatever we have partially created.

That's the same reason why we don't need to bother with failure cleanups in
the functions that populate directories - if they fail halfway through, the
entire (sub)tree is going to be wiped out in one pass.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help