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

[Al Viro <viro@zeniv.linux.org.uk>]

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.