On Wed, Dec 18, 2024 at 1:10 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Wed, Dec 18, 2024 at 1:23 AM Dmitry Safonov <dima@xxxxxxxxxx> wrote:
> >
> > Hi Amir and Miklos, linux-unionfs,
> >
> > On v6.9.0 kernel we stepped over the WARN_ON() in show_mark_fhandle():
> >
> > > ------------[ cut here ]------------
> > > Can't encode file handler for inotify: 255
> > > WARNING: CPU: 0 PID: 11136 at fs/notify/fdinfo.c:55 show_mark_fhandle+0xfa/0x110
> > > CPU: 0 PID: 11136 Comm: lsof Kdump: loaded Tainted: P W O 6.9.0 #1
> > > RIP: 0010:show_mark_fhandle+0xfa/0x110
> > > Code: 00 00 00 5b 41 5c 5d e9 44 21 97 00 80 3d 0d af 99 01 00 75 d8 89 ce 48 c7 c7 68 ad 4a 82 c6 05 fb ae 99 01 01 e8 f6 98 cc ff <0f> 0b eb bf e8 4d 29 96 00 66 66 2e 0f 1f 84 00 00 00 00 00 66 90
> > ...
> > > Call Trace:
> > > <TASK>
> > > inotify_show_fdinfo+0x124/0x170
> > > seq_show+0x188/0x1f0
> > > seq_read_iter+0x115/0x4a0
> > > seq_read+0xf9/0x130
> > > vfs_read+0xb6/0x330
> > > ksys_read+0x6b/0xf0
> > > __do_fast_syscall_32+0x80/0x110
> > > do_fast_syscall_32+0x37/0x80
> > > entry_SYSCALL_compat_after_hwframe+0x75/0x75
> > > RIP: 0023:0xf7f93569
> >
> > it later reproduced on v6.12.0. With some debug, it was narrowed down
> > to the way overlayfs encodes file handlers in ovl_encode_fh(). It
> > seems that currently it calculates them with the help of dentries.
> > Straight away from that, the reproducer becomes an easy drop_caches +
> > lsof (which parses procfs and finds some pid(s) that utilize inotify,
> > reading their correspondent fdinfo(s)).
> >
> > So, my questions are: is a dentry actually needed for
> > ovl_dentry_to_fid()? Can't it just encode fh based on an inode? It
> > seems that the only reason it "needs" a dentry is to find the origin
> > layer in ovl_check_encode_origin(), is it so?
>
...
> However, I am concerned about the possibility of exportfs_encode_fid()
> failing in fanotify_encode_fh().
>
> Most fsnotify events are generated with a reference on the dentry, but
> fsnotify_inoderemove() is called from dentry_unlink_inode() after removing
> the dentry from the inode aliases list, so does that mean that FAN_DELETE_SELF
> events from overlayfs are never reported with fid info and that we will
> always print pr_warn_ratelimited("fanotify: failed to encode fid ("...?
>
> I see that the LTP test to cover overlayfs fid events reporting (fanotify13)
> does not cover FAN_DELETE_SELF events, so I need to go check.
As predicted, I added a test case for FAN_DELETE_SELF over overlayfs
and it fails
to get the file handle of the deleted inode:
https://github.com/amir73il/ltp/commits/ovl_encode_fid/
fanotify13.c:174: TINFO: Test #6.4: FAN_REPORT_FID of delete events
with mark type FAN_MARK_INODE
[ 2967.311260] fanotify_encode_fh: 23 callbacks suppressed
[ 2967.311276] fanotify: failed to encode fid (type=0, len=0, err=-2)
[ 2967.317410] fanotify: failed to encode fid (type=0, len=0, err=-2)
[ 2967.320933] fanotify: failed to encode fid (type=0, len=0, err=-2)
fanotify13.c:268: TFAIL: handle_bytes (0) returned in event does not
equal to handle_bytes (24) returned in name_to_handle_at(2)
fanotify13.c:268: TFAIL: handle_bytes (0) returned in event does not
equal to handle_bytes (24) returned in name_to_handle_at(2)
fanotify13.c:268: TFAIL: handle_bytes (180003) returned in event does
not equal to handle_bytes (24) returned in name_to_handle_at(2)
Note that this is not a regression, because FAN_REPORT_FID was not supported on
overlayfs before 16aac5ad1fa9 ("ovl: support encoding non-decodable
file handles"),
so I do plan to fix ovl_dentry_to_fid(), but with the holidays coming
up, it could take
more time than usual.
If you have an urgency to fix the reported WARN_ONCE(), then do feel free
to post a patch to remove this assertion, because my fix to ovl_dentry_to_fid()
may be simplified to deal only with unlinked inodes, so it may not be enough
fix the use case that you reported.
Thanks,
Amir.
