All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net: socket: use BIT() for MSG_* and fix MSG_CMSG_COMPAT
@ 2021-03-21 12:39 menglong8.dong
  2021-03-21 12:39 ` [PATCH net-next 1/2] net: socket: use BIT() for MSG_* menglong8.dong
  2021-03-21 12:39 ` [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21) menglong8.dong
  0 siblings, 2 replies; 6+ messages in thread
From: menglong8.dong @ 2021-03-21 12:39 UTC (permalink / raw
  To: andy.shevchenko, kuba, linux, David.Laight
  Cc: davem, dong.menglong, viro, herbert, axboe, linux-kernel, netdev

From: Menglong Dong <dong.menglong@zte.com.cn>

In the first patch, I use BIT() for MSG_* to make the code tidier.

Directly use BIT() for MSG_* will be a bit problematic, because
'msg_flags' is defined as 'int' somewhere, and MSG_CMSG_COMPAT
will make it become negative, just like what Guenter Roeck
reported here:

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

So in the second patch, I change MSG_CMSG_COMPAT to BIT(21), as
David Laight suggested. MSG_CMSG_COMPAT is an internal value,
which is't used in userspace, so this change works.


Menglong Dong (2):
  net: socket: use BIT() for MSG_*
  net: socket: change MSG_CMSG_COMPAT to BIT(21)

 include/linux/socket.h | 72 ++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

-- 
2.31.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next 1/2] net: socket: use BIT() for MSG_*
  2021-03-21 12:39 [PATCH net-next 0/2] net: socket: use BIT() for MSG_* and fix MSG_CMSG_COMPAT menglong8.dong
@ 2021-03-21 12:39 ` menglong8.dong
  2021-03-21 12:39 ` [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21) menglong8.dong
  1 sibling, 0 replies; 6+ messages in thread
From: menglong8.dong @ 2021-03-21 12:39 UTC (permalink / raw
  To: andy.shevchenko, kuba, linux, David.Laight
  Cc: davem, dong.menglong, viro, herbert, axboe, linux-kernel, netdev

From: Menglong Dong <dong.menglong@zte.com.cn>

The bit mask for MSG_* seems a little confused here. Replace it
with BIT() to make it clear to understand.

Signed-off-by: Menglong Dong <dong.menglong@zte.com.cn>
---
 include/linux/socket.h | 71 ++++++++++++++++++++++--------------------
 1 file changed, 37 insertions(+), 34 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 385894b4a8bb..d5ebfe30d96b 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -283,42 +283,45 @@ struct ucred {
    Added those for 1003.1g not all are supported yet
  */
 
-#define MSG_OOB		1
-#define MSG_PEEK	2
-#define MSG_DONTROUTE	4
-#define MSG_TRYHARD     4       /* Synonym for MSG_DONTROUTE for DECnet */
-#define MSG_CTRUNC	8
-#define MSG_PROBE	0x10	/* Do not send. Only probe path f.e. for MTU */
-#define MSG_TRUNC	0x20
-#define MSG_DONTWAIT	0x40	/* Nonblocking io		 */
-#define MSG_EOR         0x80	/* End of record */
-#define MSG_WAITALL	0x100	/* Wait for a full request */
-#define MSG_FIN         0x200
-#define MSG_SYN		0x400
-#define MSG_CONFIRM	0x800	/* Confirm path validity */
-#define MSG_RST		0x1000
-#define MSG_ERRQUEUE	0x2000	/* Fetch message from error queue */
-#define MSG_NOSIGNAL	0x4000	/* Do not generate SIGPIPE */
-#define MSG_MORE	0x8000	/* Sender will send more */
-#define MSG_WAITFORONE	0x10000	/* recvmmsg(): block until 1+ packets avail */
-#define MSG_SENDPAGE_NOPOLICY 0x10000 /* sendpage() internal : do no apply policy */
-#define MSG_SENDPAGE_NOTLAST 0x20000 /* sendpage() internal : not the last page */
-#define MSG_BATCH	0x40000 /* sendmmsg(): more messages coming */
-#define MSG_EOF         MSG_FIN
-#define MSG_NO_SHARED_FRAGS 0x80000 /* sendpage() internal : page frags are not shared */
-#define MSG_SENDPAGE_DECRYPTED	0x100000 /* sendpage() internal : page may carry
-					  * plain text and require encryption
-					  */
-
-#define MSG_ZEROCOPY	0x4000000	/* Use user data in kernel path */
-#define MSG_FASTOPEN	0x20000000	/* Send data in TCP SYN */
-#define MSG_CMSG_CLOEXEC 0x40000000	/* Set close_on_exec for file
-					   descriptor received through
-					   SCM_RIGHTS */
+#define MSG_OOB		BIT(0)
+#define MSG_PEEK	BIT(1)
+#define MSG_DONTROUTE	BIT(2)
+#define MSG_TRYHARD	BIT(2)	/* Synonym for MSG_DONTROUTE for DECnet		*/
+#define MSG_CTRUNC	BIT(3)
+#define MSG_PROBE	BIT(4)	/* Do not send. Only probe path f.e. for MTU	*/
+#define MSG_TRUNC	BIT(5)
+#define MSG_DONTWAIT	BIT(6)	/* Nonblocking io		*/
+#define MSG_EOR		BIT(7)	/* End of record		*/
+#define MSG_WAITALL	BIT(8)	/* Wait for a full request	*/
+#define MSG_FIN		BIT(9)
+#define MSG_SYN		BIT(10)
+#define MSG_CONFIRM	BIT(11)	/* Confirm path validity	*/
+#define MSG_RST		BIT(12)
+#define MSG_ERRQUEUE	BIT(13)	/* Fetch message from error queue */
+#define MSG_NOSIGNAL	BIT(14)	/* Do not generate SIGPIPE	*/
+#define MSG_MORE	BIT(15)	/* Sender will send more	*/
+#define MSG_WAITFORONE	BIT(16)	/* recvmmsg(): block until 1+ packets avail */
+#define MSG_SENDPAGE_NOPOLICY	BIT(16)	/* sendpage() internal : do no apply policy */
+#define MSG_SENDPAGE_NOTLAST	BIT(17)	/* sendpage() internal : not the last page  */
+#define MSG_BATCH		BIT(18)	/* sendmmsg(): more messages coming */
+#define MSG_EOF	MSG_FIN
+#define MSG_NO_SHARED_FRAGS	BIT(19)	/* sendpage() internal : page frags
+					 * are not shared
+					 */
+#define MSG_SENDPAGE_DECRYPTED	BIT(20)	/* sendpage() internal : page may carry
+					 * plain text and require encryption
+					 */
+
+#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	0x80000000	/* This message needs 32 bit fixups */
+#define MSG_CMSG_COMPAT		BIT(31)	/* This message needs 32 bit fixups */
 #else
-#define MSG_CMSG_COMPAT	0		/* We never have 32 bit fixups */
+#define MSG_CMSG_COMPAT		0	/* We never have 32 bit fixups */
 #endif
 
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21)
  2021-03-21 12:39 [PATCH net-next 0/2] net: socket: use BIT() for MSG_* and fix MSG_CMSG_COMPAT menglong8.dong
  2021-03-21 12:39 ` [PATCH net-next 1/2] net: socket: use BIT() for MSG_* menglong8.dong
@ 2021-03-21 12:39 ` menglong8.dong
  2021-03-21 12:49   ` Herbert Xu
  1 sibling, 1 reply; 6+ messages in thread
From: menglong8.dong @ 2021-03-21 12:39 UTC (permalink / raw
  To: andy.shevchenko, kuba, linux, David.Laight
  Cc: davem, dong.menglong, viro, herbert, axboe, linux-kernel, netdev

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(-)

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 */
-- 
2.31.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21)
  2021-03-21 12:39 ` [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21) menglong8.dong
@ 2021-03-21 12:49   ` Herbert Xu
  2021-03-21 13:29     ` Menglong Dong
  2021-03-22  8:58     ` David Laight
  0 siblings, 2 replies; 6+ messages in thread
From: Herbert Xu @ 2021-03-21 12:49 UTC (permalink / raw
  To: menglong8.dong
  Cc: andy.shevchenko, kuba, linux, David.Laight, davem, dong.menglong,
	viro, axboe, linux-kernel, netdev

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,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21)
  2021-03-21 12:49   ` Herbert Xu
@ 2021-03-21 13:29     ` Menglong Dong
  2021-03-22  8:58     ` David Laight
  1 sibling, 0 replies; 6+ messages in thread
From: Menglong Dong @ 2021-03-21 13:29 UTC (permalink / raw
  To: Herbert Xu
  Cc: Andy Shevchenko, Jakub Kicinski, Guenter Roeck, David Laight,
	David Miller, Menglong Dong, Alexander Viro, Jens Axboe, LKML,
	netdev

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21)
  2021-03-21 12:49   ` Herbert Xu
  2021-03-21 13:29     ` Menglong Dong
@ 2021-03-22  8:58     ` David Laight
  1 sibling, 0 replies; 6+ messages in thread
From: David Laight @ 2021-03-22  8:58 UTC (permalink / raw
  To: 'Herbert Xu', menglong8.dong@gmail.com
  Cc: andy.shevchenko@gmail.com, kuba@kernel.org, linux@roeck-us.net,
	davem@davemloft.net, dong.menglong@zte.com.cn,
	viro@zeniv.linux.org.uk, axboe@kernel.dk,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org

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)


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-22  8:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-21 12:39 [PATCH net-next 0/2] net: socket: use BIT() for MSG_* and fix MSG_CMSG_COMPAT menglong8.dong
2021-03-21 12:39 ` [PATCH net-next 1/2] net: socket: use BIT() for MSG_* menglong8.dong
2021-03-21 12:39 ` [PATCH net-next 2/2] net: socket: change MSG_CMSG_COMPAT to BIT(21) menglong8.dong
2021-03-21 12:49   ` Herbert Xu
2021-03-21 13:29     ` Menglong Dong
2021-03-22  8:58     ` David Laight

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.