| Message ID | 1463114830-32751-1-git-send-email-soheil.kdev@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com> Date: Fri, 13 May 2016 00:47:10 -0400 > From: Soheil Hassas Yeganeh <soheil@google.com> > > SO_TIMESTAMP(NS), RXQ_OVFL, and WIFI_STATUS can be returned as > receive-side control messages from recvmsg(). Although invalid, > some applications may reflect those receive-side control messages > back to sendmsg(). Since socket-level control messages were being > ignored in ipv4 and ipv6, such applications would not get an error. > > 24025c4 (ipv4: process socket-level control messages in IPv4) and > ad1e46 (ipv6: process socket-level control messages in IPv6) add > support for socket-level control messages in ipv4 and ipv6 on > sendmsg(). This results in getting -EINVAL, if the application > passes in a message with SO_WIFI_STATUS, SO_RXQ_OVFL, SO_TIMESTAMP > and/or SO_TIMESTAMPNS that might have been received in recvmsg(). > > Ignore SO_WIFI_STATUS, SO_TIMESTAMP(NS), and SO_RXQ_OVFL when > processing socket-level control messages in send-side to remain > backward compatible. This patch is missing a proper Signed-Off-By: tag. But I think this change is wrong. Just because we silently accepted garbage in the past doesn't mean more strict checking is invalid. Applications blindly echoing control messages from recvmsg to sendmsg must be fixed.
On Fri, May 13, 2016 at 2:03 AM, David Miller <davem@davemloft.net> wrote: > From: Soheil Hassas Yeganeh <soheil.kdev@gmail.com> > Date: Fri, 13 May 2016 00:47:10 -0400 > >> From: Soheil Hassas Yeganeh <soheil@google.com> >> >> SO_TIMESTAMP(NS), RXQ_OVFL, and WIFI_STATUS can be returned as >> receive-side control messages from recvmsg(). Although invalid, >> some applications may reflect those receive-side control messages >> back to sendmsg(). Since socket-level control messages were being >> ignored in ipv4 and ipv6, such applications would not get an error. >> >> 24025c4 (ipv4: process socket-level control messages in IPv4) and >> ad1e46 (ipv6: process socket-level control messages in IPv6) add >> support for socket-level control messages in ipv4 and ipv6 on >> sendmsg(). This results in getting -EINVAL, if the application >> passes in a message with SO_WIFI_STATUS, SO_RXQ_OVFL, SO_TIMESTAMP >> and/or SO_TIMESTAMPNS that might have been received in recvmsg(). >> >> Ignore SO_WIFI_STATUS, SO_TIMESTAMP(NS), and SO_RXQ_OVFL when >> processing socket-level control messages in send-side to remain >> backward compatible. > > This patch is missing a proper Signed-Off-By: tag. Oops, sorry. :( > But I think this change is wrong. Just because we silently accepted > garbage in the past doesn't mean more strict checking is invalid. > > Applications blindly echoing control messages from recvmsg to sendmsg > must be fixed. Agreed, by this patch, I just wanted to note such potential issues and make sure I haven't broken any user-space application. Thanks David!
Hello. On 5/13/2016 7:47 AM, Soheil Hassas Yeganeh wrote: > From: Soheil Hassas Yeganeh <soheil@google.com> > > SO_TIMESTAMP(NS), RXQ_OVFL, and WIFI_STATUS can be returned as > receive-side control messages from recvmsg(). Although invalid, > some applications may reflect those receive-side control messages > back to sendmsg(). Since socket-level control messages were being > ignored in ipv4 and ipv6, such applications would not get an error. > > 24025c4 (ipv4: process socket-level control messages in IPv4) and > ad1e46 (ipv6: process socket-level control messages in IPv6) add scripts/checkpatch.pl now enforces certain commit citing style, yours doesn't quite match -- SHA1 should be at least 12 digits and the summary enclosed in (""). > support for socket-level control messages in ipv4 and ipv6 on > sendmsg(). This results in getting -EINVAL, if the application > passes in a message with SO_WIFI_STATUS, SO_RXQ_OVFL, SO_TIMESTAMP > and/or SO_TIMESTAMPNS that might have been received in recvmsg(). > > Ignore SO_WIFI_STATUS, SO_TIMESTAMP(NS), and SO_RXQ_OVFL when > processing socket-level control messages in send-side to remain > backward compatible. > --- > net/core/sock.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/core/sock.c b/net/core/sock.c > index 08bf97e..1e0bcd0 100644 > --- a/net/core/sock.c > +++ b/net/core/sock.c > @@ -1938,6 +1938,12 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, > sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; > sockc->tsflags |= tsflags; > break; > + /* Ignore the following types on send to remain backward compatible. */ > + case SO_RXQ_OVFL: /* Fall through */ > + case SO_TIMESTAMP: /* Fall through */ > + case SO_TIMESTAMPNS: /* Fall through */ ^^^^^^^^^^^^^^^^^^ There's no need for such comments here. > + case SO_WIFI_STATUS: > + break; > default: > return -EINVAL; > } MBR, Sergei
diff --git a/net/core/sock.c b/net/core/sock.c index 08bf97e..1e0bcd0 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1938,6 +1938,12 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; sockc->tsflags |= tsflags; break; + /* Ignore the following types on send to remain backward compatible. */ + case SO_RXQ_OVFL: /* Fall through */ + case SO_TIMESTAMP: /* Fall through */ + case SO_TIMESTAMPNS: /* Fall through */ + case SO_WIFI_STATUS: + break; default: return -EINVAL; }
From: Soheil Hassas Yeganeh <soheil@google.com> SO_TIMESTAMP(NS), RXQ_OVFL, and WIFI_STATUS can be returned as receive-side control messages from recvmsg(). Although invalid, some applications may reflect those receive-side control messages back to sendmsg(). Since socket-level control messages were being ignored in ipv4 and ipv6, such applications would not get an error. 24025c4 (ipv4: process socket-level control messages in IPv4) and ad1e46 (ipv6: process socket-level control messages in IPv6) add support for socket-level control messages in ipv4 and ipv6 on sendmsg(). This results in getting -EINVAL, if the application passes in a message with SO_WIFI_STATUS, SO_RXQ_OVFL, SO_TIMESTAMP and/or SO_TIMESTAMPNS that might have been received in recvmsg(). Ignore SO_WIFI_STATUS, SO_TIMESTAMP(NS), and SO_RXQ_OVFL when processing socket-level control messages in send-side to remain backward compatible. --- net/core/sock.c | 6 ++++++ 1 file changed, 6 insertions(+)