On Thu, 26 Mar 2026, Chuck Lever wrote: > > On Wed, Mar 25, 2026, at 3:08 AM, NeilBrown wrote: > > On Wed, 25 Mar 2026, Chuck Lever wrote: > >> > >> On Tue, Mar 24, 2026, at 6:13 AM, NeilBrown wrote: > >> > From: NeilBrown <[email protected]> > >> > > >> > The F_GETLK fcntl can work with either read access or write access or > >> > both. It can query F_RDLCK and F_WRLCK locks in either case. > >> > > >> > However lockd currently treats F_GETLK similar to F_SETLK in that read > >> > access is required to query an F_RDLCK lock and write access is required > >> > to query a F_WRLCK lock. > >> > > >> > This is wrong and can cause problem - e.g. when qemu accesses a > >> > read-only (e.g. iso) filesystem image over NFS (though why it queries > >> > if it can get a write lock - I don't know. But it does, and this works > >> > with local filesystems). > >> > > >> > So we need TEST requests to be handled differently. To do this: > >> > > >> > - change nlm_do_fopen() to accept O_RDWR as a mode and in that case > >> > succeed if either a O_RDONLY or O_WRONLY file can be opened. > >> > - change nlm_lookup_file() to accept a mode argument from caller, > >> > instead of deducing base on lock time, and pass that on to > >> > nlm_do_fopen() > >> > - change nlm4svc_retrieve_args() and nlmsvc_retrieve_args() to detect > >> > TEST requests and pass O_RDWR as a mode to nlm_lookup_file, passing > >> > the same mode as before for other requests. Also set > >> > lock->fl.c.flc_file to whichever file is available for TEST requests. > >> > - change nlmsvc_testlock() to also not calculate the mode, but to use > >> > whenever was stored in lock->fl.c.flc_file. > >> > > >> > Reported-by: Tj <[email protected]> > >> > Link: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1128861 > >> > Fixes: 7f024fcd5c97 ("Keep read and write fds with each nlm_file") > >> > Signed-off-by: NeilBrown <[email protected]> > >> > >> Hi Neil, which kernels should this fix apply to? > >> > > > > v6.13 and later. So linux-6.18.y and linux-6.19.y > > Assuming that includes upstream, I recommend that I take this > into nfsd-testing / nfsd-next and let nature, ah, er, stable > automation, take it's course. > > > > The Fixes: tag is actually wrong. This bug has been present forever. > > However a different bug that > > Commit: 4cc9b9f2bf4d ("nfsd: refine and rename NFSD_MAY_LOCK") > > fixed was hiding the bug. > > > > So it should probably be marked > > Fixes: 4cc9b9f2bf4d ("nfsd: refine and rename NFSD_MAY_LOCK") > > with an explanation. > > IIUC, we want Fixes: to point to the commit that introduced > the issue (Fixes: since forever) and then use a "# v6.13+" > comment on the Cc: stable to control how far back to backport > it. > > Commit message could mention that 4cc9b9f2bf4d uncovered the > issue.
What you suggest is correct in a pure sense (and I like that) but as Ben points out, it will miss older kernels that have backported 4cc9b9f2bf4d. We know which kernel.org kernels that includes, but not what other organisations might maintain for their own purposes. So I think we meet the needs of automation best by saying: Fixes: 4cc9b9f2bf4d even though that didn't introduce the bug but only expose the bug. NeilBrown
