From: Menglong Dong <redacted>
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.
In version 2, some comment is added in patch 2 to stop people
from using BIT(31) for MSG_* in the feature, as Herbert Xu
suggested.
Menglong Dong (2):
net: socket: use BIT() for MSG_*
net: socket: change MSG_CMSG_COMPAT to BIT(21)
include/linux/socket.h | 75 +++++++++++++++++++++++-------------------
1 file changed, 41 insertions(+), 34 deletions(-)
--
2.31.0
From: Menglong Dong <redacted>
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 <redacted>
---
include/linux/socket.h | 71 ++++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 34 deletions(-)
@@ -283,42 +283,45 @@ struct ucred {Addedthosefor1003.1gnotallaresupportedyet*/-#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-*plaintextandrequireencryption-*/--#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-descriptorreceivedthrough-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+*arenotshared+*/+#define MSG_SENDPAGE_DECRYPTED BIT(20) /* sendpage() internal : page may carry+*plaintextandrequireencryption+*/++#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+*descriptorreceivedthrough+*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
From: Menglong Dong <redacted>
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 <redacted>
---
v2:
- add comment to stop people from trying to use BIT(31)
---
include/linux/socket.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -312,17 +312,21 @@ struct ucred {*plaintextandrequireencryption*/+#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*descriptorreceivedthrough*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+/* Attention: Don't use BIT(31) for MSG_*, as 'msg_flags' is defined+*as'int'somewhereandBIT(31)willmakeitbecomenegative.+*//* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */
On 3/21/21 6:43 AM, menglong8.dong@gmail.com wrote:
From: Menglong Dong <redacted>
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 <redacted>
With this patch sent as patch 1/2, any code trying to bisect
a compat related network problem will fail at this commit.
Guenter
@@ -283,42 +283,45 @@ struct ucred {Addedthosefor1003.1gnotallaresupportedyet*/-#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-*plaintextandrequireencryption-*/--#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-descriptorreceivedthrough-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+*arenotshared+*/+#define MSG_SENDPAGE_DECRYPTED BIT(20) /* sendpage() internal : page may carry+*plaintextandrequireencryption+*/++#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+*descriptorreceivedthrough+*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
On 3/21/21 6:43 AM, menglong8.dong@gmail.com wrote:
From: Menglong Dong <redacted>
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.
Maybe I am overly concerned (or maybe call it pessimistic),
but I do wonder if this change is worth the added risk. Personally
I'd rather start changing all 'int flag' uses to 'unsigned int flag'
and, only after that is complete, make the switch to BIT().
Of course, that is just my personal opinion, and, as I said,
I may be overly concerned.
Guenter
quoted hunk
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Menglong Dong <redacted>
---
v2:
- add comment to stop people from trying to use BIT(31)
---
include/linux/socket.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
@@ -312,17 +312,21 @@ struct ucred {*plaintextandrequireencryption*/+#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*descriptorreceivedthrough*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+/* Attention: Don't use BIT(31) for MSG_*, as 'msg_flags' is defined+*as'int'somewhereandBIT(31)willmakeitbecomenegative.+*//* Setsockoptions(2) level. Thanks to BSD these must match IPPROTO_xxx */