Re: [PATCH v2 08/12] cachefiles: never get a new anonymous fd if ondemand_id is valid

[Gao Xiang]

On 2024/5/20 19:14, Baokun Li wrote:
> On 2024/5/20 17:24, Jingbo Xu wrote:
> > On 5/20/24 5:07 PM, Baokun Li wrote:
> > > On 2024/5/20 16:43, Jingbo Xu wrote:
> > > > On 5/15/24 4:45 PM, libaokun@xxxxxxxxxxxxxxx wrote:
> > > > > From: Baokun Li <libaokun1@xxxxxxxxxx>
> SNIP
> > > > > To avoid this, allocate a new anonymous fd only if no anonymous fd has
> > > > > been allocated (ondemand_id == 0) or if the previously allocated
> > > > > anonymous
> > > > > fd has been closed (ondemand_id == -1). Moreover, returns an error if
> > > > > ondemand_id is valid, letting the daemon know that the current userland
> > > > > restore logic is abnormal and needs to be checked.
> > > > >
> > > > > Fixes: c8383054506c ("cachefiles: notify the user daemon when looking
> > > > > up cookie")
> > > > > Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
> > > > The LOCs of this fix is quite under control.  But still it seems that
> > > > the worst consequence is that the (potential) malicious daemon gets
> > > > hung.  No more effect to the system or other processes.  Or does a
> > > > non-malicious daemon have any chance having the same issue?
> > > If we enable hung_task_panic, it may cause panic to crash the server.
> > Then this issue has nothing to do with this patch?  As long as a
> > malicious daemon doesn't close the anonymous fd after umounting, then I
> > guess a following attempt of mounting cookie with the same name will
> > also wait and hung there?
> Yes, a daemon that only reads requests but doesn't process them will
> cause hung，but the daemon will obey the basic constraints when we
> test it.

If we'd really like to enhanace this ("hung_task_panic"), I think
you'd better to switch wait_for_completion() to
wait_for_completion_killable() at least IMHO anyway.

Thanks,
Gao Xiang