diff mbox series

[net-next,2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21)

Message ID 20210321123929.142838-3-dong.menglong@zte.com.cn
State New
Headers show
Series net: socket: use BIT() for MSG_* and fix MSG_CMSG_COMPAT | expand

Commit Message

Menglong Dong March 21, 2021, 12:39 p.m. UTC 
From: Menglong Dong <dong.menglong@zte.com.cn>

Currently, MSG_CMSG_COMPAT is defined as '1 << 31'. However, 'msg_flags'
is defined with type of 'int' somewhere, such as 'packet_recvmsg' and
other recvmsg functions:

static int packet_recvmsg(struct socket *sock, struct msghdr *msg,
			  size_t len,
			  int flags)

If MSG_CMSG_COMPAT is set in 'flags', it's value will be negative.
Once it perform bit operations with MSG_*, the upper 32 bits of
the result will be set, just like what Guenter Roeck explained
here:

https://lore.kernel.org/netdev/20210317013758.GA134033@roeck-us.net

As David Laight suggested, fix this by change MSG_CMSG_COMPAT to
some value else. MSG_CMSG_COMPAT is an internal value, which is't
used in userspace, so this change works.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn>
---
 include/linux/socket.h | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Herbert Xu March 21, 2021, 12:49 p.m. UTC | #1 
On Sun, Mar 21, 2021 at 08:39:29PM +0800, menglong8.dong@gmail.com wrote:
>
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index d5ebfe30d96b..317b2933f499 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -312,17 +312,18 @@ struct ucred {
>  					 * plain text and require encryption
>  					 */
>  
> +#if defined(CONFIG_COMPAT)
> +#define MSG_CMSG_COMPAT		BIT(21)	/* This message needs 32 bit fixups */
> +#else
> +#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */
> +#endif
> +
>  #define MSG_ZEROCOPY		BIT(26)	/* Use user data in kernel path */
>  #define MSG_FASTOPEN		BIT(29)	/* Send data in TCP SYN */
>  #define MSG_CMSG_CLOEXEC	BIT(30)	/* Set close_on_exec for file
>  					 * descriptor received through
>  					 * SCM_RIGHTS
>  					 */
> -#if defined(CONFIG_COMPAT)
> -#define MSG_CMSG_COMPAT		BIT(31)	/* This message needs 32 bit fixups */
> -#else
> -#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */
> -#endif

Shouldn't you add some comment here to stop people from trying to
use BIT(31) in the future?

Thanks,
Menglong Dong March 21, 2021, 1:29 p.m. UTC | #2 
On Sun, Mar 21, 2021 at 8:49 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
...
>
> Shouldn't you add some comment here to stop people from trying to
> use BIT(31) in the future?
>
> Thanks,

Yeah, I think it's necessary. Thank you for your reminder~

With Regards,
Menglong Dong
David Laight March 22, 2021, 8:58 a.m. UTC | #3 
From: Herbert Xu

> Sent: 21 March 2021 12:49

> 

> On Sun, Mar 21, 2021 at 08:39:29PM +0800, menglong8.dong@gmail.com wrote:

> >

> > diff --git a/include/linux/socket.h b/include/linux/socket.h

> > index d5ebfe30d96b..317b2933f499 100644

> > --- a/include/linux/socket.h

> > +++ b/include/linux/socket.h

> > @@ -312,17 +312,18 @@ struct ucred {

> >  					 * plain text and require encryption

> >  					 */

> >

> > +#if defined(CONFIG_COMPAT)

> > +#define MSG_CMSG_COMPAT		BIT(21)	/* This message needs 32 bit fixups */

> > +#else

> > +#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */

> > +#endif

> > +

> >  #define MSG_ZEROCOPY		BIT(26)	/* Use user data in kernel path */

> >  #define MSG_FASTOPEN		BIT(29)	/* Send data in TCP SYN */

> >  #define MSG_CMSG_CLOEXEC	BIT(30)	/* Set close_on_exec for file

> >  					 * descriptor received through

> >  					 * SCM_RIGHTS

> >  					 */

> > -#if defined(CONFIG_COMPAT)

> > -#define MSG_CMSG_COMPAT		BIT(31)	/* This message needs 32 bit fixups */

> > -#else

> > -#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */

> > -#endif

> 

> Shouldn't you add some comment here to stop people from trying to

> use BIT(31) in the future?


You'd also be better using BIT(30) - ie the other end of the
free space from the user-visible bits.

It has to be said that the entire impossibility of writing BIT(n)
safely almost makes it worse that just defining appropriate constants.

Personally I like the hex constants.
The make it much easier to work out which bits are set in a diagnostic
print (or memory hexdump).

The only time I've really found BIT() type macros useful is when
defining values that have to match hardware specs that define bit
numbers backwards starting from 1.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/include/linux/socket.h b/include/linux/socket.h
index d5ebfe30d96b..317b2933f499 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -312,17 +312,18 @@  struct ucred {
 					 * plain text and require encryption
 					 */
 
+#if defined(CONFIG_COMPAT)
+#define MSG_CMSG_COMPAT		BIT(21)	/* This message needs 32 bit fixups */
+#else
+#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */
+#endif
+
 #define MSG_ZEROCOPY		BIT(26)	/* Use user data in kernel path */
 #define MSG_FASTOPEN		BIT(29)	/* Send data in TCP SYN */
 #define MSG_CMSG_CLOEXEC	BIT(30)	/* Set close_on_exec for file
 					 * descriptor received through
 					 * SCM_RIGHTS
 					 */
-#if defined(CONFIG_COMPAT)
-#define MSG_CMSG_COMPAT		BIT(31)	/* This message needs 32 bit fixups */
-#else
-#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */
-#endif
 
 
 /* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */