Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Rolf Eike Beer <eike-kernel@sf-tec.de>]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

Am Freitag, 23. Oktober 2020, 10:31:14 CEST schrieb Helge Deller:
> On 10/23/20 10:18 AM, Helge Deller wrote:
> > On 10/23/20 9:25 AM, Rolf Eike Beer wrote:
> >>> +#define O_NONBLOCK_OLD		000200004
> >>> +#define O_NONBLOCK_MASK_OUT	(O_NONBLOCK_OLD & ~O_NONBLOCK)
> >>> +static int FIX_O_NONBLOCK(int flags)
> >>> +{
> >>> +	if (flags & O_NONBLOCK_MASK_OUT) {
> >>> +		struct task_struct *tsk = current;
> >>> +		pr_warn("%s(%d) uses old O_NONBLOCK value. "
> >>> +			"Please recompile the application.\n",
> >>> +			tsk->comm, tsk->pid);
> >>> +	}
> >>> +	return flags & ~O_NONBLOCK_MASK_OUT;
> >>> +}
> >> 
> >> This will also trigger if I just pass 0x4 in flags, no? The check should
> >> be
> >> 
> >> 	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)
> > 
> > RIGHT!
> > That's a very good point.
> > I was thinking about what would happen if over time a new (unrelated)
> > define gets created which then gets 0x4 as value. My code would then have
> > wrongly modified it.
> 
> After some more thinking....
> 
> It's not that easy.
> Let's assume there will be another new flag at some time with value 0x4.
> Now, the caller sets this flag (0x4) and new O_NONBLOCK (000200000),
> so you end up with 000200004 again, which then triggers your check:
>  	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)
> 
> So, my check to test only the mask for 0x4 was basically better,
> because it would prevent the usage of 0x4 as any new value.
> In any way, it seems we need to avoid using 0x4 for a long time...

Then maybe just add that as an explicit comment in the header so noone picks 
it by accident, and add a comment pointing to these wrappers so they are more 
likely to be kept in sync.

And for extra fun we can just scrap all this for 64 bit userspace as we don't 
need the compat there ;)

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]