Subject: Re: [PATCH] compat bug in sys_recvmsg, MSG_CMSG_COMPAT check missing

From: Andreas Schwab <[email-masked]>
Date: Sun, 06 Jun 2004 00:05:58 +0200

"David S. Miller" <[email-masked]> writes:

> On Sat, 05 Jun 2004 23:47:22 +0200
> Andreas Schwab <[email-masked]> wrote:
>
>> > Olaf's patch, it said:
>> >
>> > -	if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC))
>> > +	if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
>> 
>> Yes, and where is the problem?
>
> If MSG_CMSG_COMPAT is "ZERO", which it will be if CONFIG_COMPAT is
> not set, then "~0" is all bits, therefore if any bit (even the ones
> we want to accept) is set we will return failure.  The test ends
> up amounting to:
>
> 	if (flags & ~0)
>
> which is true if any bit is set, that's not what we want.

Can you say DeMorgan?

> diff -Nru a/include/linux/socket.h b/include/linux/socket.h
> --- a/include/linux/socket.h	2004-06-05 14:53:34 -07:00
> +++ b/include/linux/socket.h	2004-06-05 14:53:34 -07:00
> @@ -241,8 +241,10 @@
>  
>  #if defined(CONFIG_COMPAT)
>  #define MSG_CMSG_COMPAT	0x80000000	/* This message needs 32 bit fixups */
> +#define MSG_FLAGS_USER(X)	((X) & ~MSG_CMSG_COMPAT)
>  #else
>  #define MSG_CMSG_COMPAT	0		/* We never have 32 bit fixups */
> +#define MSG_FLAGS_USER(X)	(X)
>  #endif
>  
>  
> diff -Nru a/net/appletalk/ddp.c b/net/appletalk/ddp.c
> --- a/net/appletalk/ddp.c	2004-06-05 14:53:35 -07:00
> +++ b/net/appletalk/ddp.c	2004-06-05 14:53:35 -07:00
> @@ -1567,7 +1567,7 @@
>  	struct atalk_route *rt;
>  	int err;
>  
> -	if (flags & ~MSG_DONTWAIT)
> +	if (MSG_FLAGS_USER(flags) & ~MSG_DONTWAIT)
>  		return -EINVAL;
>  
>  	if (len > DDP_MAXSZ)

This is exactly equivalent to Olaf's version.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [email-masked]
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."