linux-kernel - Re: [RFC PATCH] file: Wrap locking mechanism for f_pos

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z1B2Lxxenie3SA6d@vaxr-BM6660-BM6360>
Date: Wed, 4 Dec 2024 23:33:03 +0800
From: I Hsin Cheng <richard120310@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, brauner@...nel.org
Subject: Re: [RFC PATCH] file: Wrap locking mechanism for f_pos_lock

On Wed, Dec 04, 2024 at 01:48:29PM +0100, Jan Kara wrote:
> On Wed 04-12-24 12:11:02, Christian Brauner wrote:
> > > motivation of introducing __f_unlock_pos() in the first place? It has one
> > 
> > May I venture a guess:
> > 
> >   CALL    ../scripts/checksyscalls.sh
> >   INSTALL libsubcmd_headers
> >   INSTALL libsubcmd_headers
> >   CC      fs/read_write.o
> > In file included from ../fs/read_write.c:12:
> > ../include/linux/file.h:78:27: error: incomplete definition of type 'struct file'
> >    78 |                 mutex_unlock(&fd_file(f)->f_pos_lock);
> >       |                               ~~~~~~~~~~^
> > 
> > If you don't include linux/fs.h before linux/file.h you'd get compilation
> > errors and we don't want to include linux/fs.h in linux/file.h.
> 
> Ah, subtle ;)
> 
> > I wouldn't add another wrapper for lock though. Just put a comment on top of
> > __f_unlock_pos().       
> 
> Yes, I guess comment is better in that case.
> 
> 								Honza
> 
> -- 
> Jan Kara <jack@...e.com>
> SUSE Labs, CR


No problem, I'll add comments on __f_unlock_pos() to explain for the
inconsistency then send a formal path.

But I want to ask what's the motivation of defining "fdput_pos()" as
static inline? If we make it "void fdput_pos()", we should be able to
write the implementation in file.c and thus can get rid of
"__f_unlock_pos()".

Is it just for the inline function speed up?

Best regards,
Richard Cheng.