| Message ID | 20200728015505.37830-1-decui@microsoft.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | [for,v4.4,only] udp: drop corrupt packets earlier to avoid data corruption | expand |
> From: Dexuan Cui <decui@microsoft.com> > Sent: Monday, July 27, 2020 6:55 PM > To: gregkh@linuxfoundation.org; edumazet@google.com; > stable@vger.kernel.org > Cc: w@1wt.eu; Dexuan Cui <decui@microsoft.com>; Joseph Salisbury > <Joseph.Salisbury@microsoft.com>; Michael Kelley <mikelley@microsoft.com>; > viro@zeniv.linux.org.uk; netdev@vger.kernel.org; davem@davemloft.net; > ohering@suse.com > Subject: [PATCH][for v4.4 only] udp: drop corrupt packets earlier to avoid data > corruption > > The v4.4 stable kernel lacks this bugfix: > commit 327868212381 ("make skb_copy_datagram_msg() et.al. preserve > ->msg_iter on error"). > As a result, the v4.4 kernel can deliver corrupt data to the application > when a corrupt UDP packet is closely followed by a valid UDP packet: the > same invocation of the recvmsg() syscall can deliver the corrupt packet's > UDP payload to the application with the UDP payload length and the > "from IP/Port" of the valid packet. > > Details: > > For a UDP packet longer than 76 bytes (see the v5.8-rc6 kernel's > include/linux/skbuff.h:3951), Linux delays the UDP checksum verification > until the application invokes the syscall recvmsg(). > > In the recvmsg() syscall handler, while Linux is copying the UDP payload > to the application's memory, it calculates the UDP checksum. If the > calculated checksum doesn't match the received checksum, Linux drops the > corrupt UDP packet, and then starts to process the next packet (if any), > and if the next packet is valid (i.e. the checksum is correct), Linux > will copy the valid UDP packet's payload to the application's receiver > buffer. > > The bug is: before Linux starts to copy the valid UDP packet, the data > structure used to track how many more bytes should be copied to the > application memory is not reset to what it was when the application just > entered the kernel by the syscall! Consequently, only a small portion or > none of the valid packet's payload is copied to the application's > receive buffer, and later when the application exits from the kernel, > actually most of the application's receive buffer contains the payload > of the corrupt packet while recvmsg() returns the length of the UDP > payload of the valid packet. > > For the mainline kernel, the bug was fixed in commit 327868212381, > but unluckily the bugfix is only backported to v4.9+. It turns out > backporting 327868212381 to v4.4 means that some supporting patches > must be backported first, so the overall changes seem too big, so the > alternative is performs the csum validation earlier and drops the > corrupt packets earlier. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Dexuan Cui <decui@microsoft.com> > --- > net/ipv4/udp.c | 3 +-- > net/ipv6/udp.c | 6 ++---- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index bb30699..49ab587 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct > sk_buff *skb) > } > } > > - if (rcu_access_pointer(sk->sk_filter) && > - udp_lib_checksum_complete(skb)) > + if (udp_lib_checksum_complete(skb)) > goto csum_error; > > if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index 73f1112..2d6703d 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -686,10 +686,8 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct > sk_buff *skb) > } > } > > - if (rcu_access_pointer(sk->sk_filter)) { > - if (udp_lib_checksum_complete(skb)) > - goto csum_error; > - } > + if (udp_lib_checksum_complete(skb)) > + goto csum_error; > > if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { > UDP6_INC_STATS_BH(sock_net(sk), > -- > 1.8.3.1 +Sasha This patch is targeted to the linux-4.4.y branch of the stable tree. Thanks, -- Dexuan
On Fri, Aug 07, 2020 at 06:03:00PM +0000, Dexuan Cui wrote: >> From: Dexuan Cui <decui@microsoft.com> >> Sent: Monday, July 27, 2020 6:55 PM >> To: gregkh@linuxfoundation.org; edumazet@google.com; >> stable@vger.kernel.org >> Cc: w@1wt.eu; Dexuan Cui <decui@microsoft.com>; Joseph Salisbury >> <Joseph.Salisbury@microsoft.com>; Michael Kelley <mikelley@microsoft.com>; >> viro@zeniv.linux.org.uk; netdev@vger.kernel.org; davem@davemloft.net; >> ohering@suse.com >> Subject: [PATCH][for v4.4 only] udp: drop corrupt packets earlier to avoid data >> corruption >> >> The v4.4 stable kernel lacks this bugfix: >> commit 327868212381 ("make skb_copy_datagram_msg() et.al. preserve >> ->msg_iter on error"). >> As a result, the v4.4 kernel can deliver corrupt data to the application >> when a corrupt UDP packet is closely followed by a valid UDP packet: the >> same invocation of the recvmsg() syscall can deliver the corrupt packet's >> UDP payload to the application with the UDP payload length and the >> "from IP/Port" of the valid packet. >> >> Details: >> >> For a UDP packet longer than 76 bytes (see the v5.8-rc6 kernel's >> include/linux/skbuff.h:3951), Linux delays the UDP checksum verification >> until the application invokes the syscall recvmsg(). >> >> In the recvmsg() syscall handler, while Linux is copying the UDP payload >> to the application's memory, it calculates the UDP checksum. If the >> calculated checksum doesn't match the received checksum, Linux drops the >> corrupt UDP packet, and then starts to process the next packet (if any), >> and if the next packet is valid (i.e. the checksum is correct), Linux >> will copy the valid UDP packet's payload to the application's receiver >> buffer. >> >> The bug is: before Linux starts to copy the valid UDP packet, the data >> structure used to track how many more bytes should be copied to the >> application memory is not reset to what it was when the application just >> entered the kernel by the syscall! Consequently, only a small portion or >> none of the valid packet's payload is copied to the application's >> receive buffer, and later when the application exits from the kernel, >> actually most of the application's receive buffer contains the payload >> of the corrupt packet while recvmsg() returns the length of the UDP >> payload of the valid packet. >> >> For the mainline kernel, the bug was fixed in commit 327868212381, >> but unluckily the bugfix is only backported to v4.9+. It turns out >> backporting 327868212381 to v4.4 means that some supporting patches >> must be backported first, so the overall changes seem too big, so the >> alternative is performs the csum validation earlier and drops the >> corrupt packets earlier. >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Signed-off-by: Dexuan Cui <decui@microsoft.com> >> --- >> net/ipv4/udp.c | 3 +-- >> net/ipv6/udp.c | 6 ++---- >> 2 files changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c >> index bb30699..49ab587 100644 >> --- a/net/ipv4/udp.c >> +++ b/net/ipv4/udp.c >> @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct >> sk_buff *skb) >> } >> } >> >> - if (rcu_access_pointer(sk->sk_filter) && >> - udp_lib_checksum_complete(skb)) >> + if (udp_lib_checksum_complete(skb)) >> goto csum_error; >> >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c >> index 73f1112..2d6703d 100644 >> --- a/net/ipv6/udp.c >> +++ b/net/ipv6/udp.c >> @@ -686,10 +686,8 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct >> sk_buff *skb) >> } >> } >> >> - if (rcu_access_pointer(sk->sk_filter)) { >> - if (udp_lib_checksum_complete(skb)) >> - goto csum_error; >> - } >> + if (udp_lib_checksum_complete(skb)) >> + goto csum_error; >> >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { >> UDP6_INC_STATS_BH(sock_net(sk), >> -- >> 1.8.3.1 > >+Sasha > >This patch is targeted to the linux-4.4.y branch of the stable tree. Eric, will you ack this (or have a missed a previous ack)?
On Wed, Aug 12, 2020 at 5:06 PM Sasha Levin <sashal@kernel.org> wrote: > > On Fri, Aug 07, 2020 at 06:03:00PM +0000, Dexuan Cui wrote: > >> From: Dexuan Cui <decui@microsoft.com> > >> Sent: Monday, July 27, 2020 6:55 PM > >> To: gregkh@linuxfoundation.org; edumazet@google.com; > >> stable@vger.kernel.org > >> Cc: w@1wt.eu; Dexuan Cui <decui@microsoft.com>; Joseph Salisbury > >> <Joseph.Salisbury@microsoft.com>; Michael Kelley <mikelley@microsoft.com>; > >> viro@zeniv.linux.org.uk; netdev@vger.kernel.org; davem@davemloft.net; > >> ohering@suse.com > >> Subject: [PATCH][for v4.4 only] udp: drop corrupt packets earlier to avoid data > >> corruption > >> > >> The v4.4 stable kernel lacks this bugfix: > >> commit 327868212381 ("make skb_copy_datagram_msg() et.al. preserve > >> ->msg_iter on error"). > >> As a result, the v4.4 kernel can deliver corrupt data to the application > >> when a corrupt UDP packet is closely followed by a valid UDP packet: the > >> same invocation of the recvmsg() syscall can deliver the corrupt packet's > >> UDP payload to the application with the UDP payload length and the > >> "from IP/Port" of the valid packet. > >> > >> Details: > >> > >> For a UDP packet longer than 76 bytes (see the v5.8-rc6 kernel's > >> include/linux/skbuff.h:3951), Linux delays the UDP checksum verification > >> until the application invokes the syscall recvmsg(). > >> > >> In the recvmsg() syscall handler, while Linux is copying the UDP payload > >> to the application's memory, it calculates the UDP checksum. If the > >> calculated checksum doesn't match the received checksum, Linux drops the > >> corrupt UDP packet, and then starts to process the next packet (if any), > >> and if the next packet is valid (i.e. the checksum is correct), Linux > >> will copy the valid UDP packet's payload to the application's receiver > >> buffer. > >> > >> The bug is: before Linux starts to copy the valid UDP packet, the data > >> structure used to track how many more bytes should be copied to the > >> application memory is not reset to what it was when the application just > >> entered the kernel by the syscall! Consequently, only a small portion or > >> none of the valid packet's payload is copied to the application's > >> receive buffer, and later when the application exits from the kernel, > >> actually most of the application's receive buffer contains the payload > >> of the corrupt packet while recvmsg() returns the length of the UDP > >> payload of the valid packet. > >> > >> For the mainline kernel, the bug was fixed in commit 327868212381, > >> but unluckily the bugfix is only backported to v4.9+. It turns out > >> backporting 327868212381 to v4.4 means that some supporting patches > >> must be backported first, so the overall changes seem too big, so the > >> alternative is performs the csum validation earlier and drops the > >> corrupt packets earlier. > >> > >> Signed-off-by: Eric Dumazet <edumazet@google.com> > >> Signed-off-by: Dexuan Cui <decui@microsoft.com> > >> --- > >> net/ipv4/udp.c | 3 +-- > >> net/ipv6/udp.c | 6 ++---- > >> 2 files changed, 3 insertions(+), 6 deletions(-) > >> > >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > >> index bb30699..49ab587 100644 > >> --- a/net/ipv4/udp.c > >> +++ b/net/ipv4/udp.c > >> @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct > >> sk_buff *skb) > >> } > >> } > >> > >> - if (rcu_access_pointer(sk->sk_filter) && > >> - udp_lib_checksum_complete(skb)) > >> + if (udp_lib_checksum_complete(skb)) > >> goto csum_error; > >> > >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { > >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > >> index 73f1112..2d6703d 100644 > >> --- a/net/ipv6/udp.c > >> +++ b/net/ipv6/udp.c > >> @@ -686,10 +686,8 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct > >> sk_buff *skb) > >> } > >> } > >> > >> - if (rcu_access_pointer(sk->sk_filter)) { > >> - if (udp_lib_checksum_complete(skb)) > >> - goto csum_error; > >> - } > >> + if (udp_lib_checksum_complete(skb)) > >> + goto csum_error; > >> > >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { > >> UDP6_INC_STATS_BH(sock_net(sk), > >> -- > >> 1.8.3.1 > > > >+Sasha > > > >This patch is targeted to the linux-4.4.y branch of the stable tree. > > Eric, will you ack this (or have a missed a previous ack)? Sure, although I have already a Signed-off-by: tag on this one, since I wrote this simpler fix for stable. If needed : Acked-by: Eric Dumazet <edumazet@google.com> Thanks.
On Wed, Aug 12, 2020 at 05:09:37PM -0700, Eric Dumazet wrote: >On Wed, Aug 12, 2020 at 5:06 PM Sasha Levin <sashal@kernel.org> wrote: >> >> On Fri, Aug 07, 2020 at 06:03:00PM +0000, Dexuan Cui wrote: >> >> From: Dexuan Cui <decui@microsoft.com> >> >> Sent: Monday, July 27, 2020 6:55 PM >> >> To: gregkh@linuxfoundation.org; edumazet@google.com; >> >> stable@vger.kernel.org >> >> Cc: w@1wt.eu; Dexuan Cui <decui@microsoft.com>; Joseph Salisbury >> >> <Joseph.Salisbury@microsoft.com>; Michael Kelley <mikelley@microsoft.com>; >> >> viro@zeniv.linux.org.uk; netdev@vger.kernel.org; davem@davemloft.net; >> >> ohering@suse.com >> >> Subject: [PATCH][for v4.4 only] udp: drop corrupt packets earlier to avoid data >> >> corruption >> >> >> >> The v4.4 stable kernel lacks this bugfix: >> >> commit 327868212381 ("make skb_copy_datagram_msg() et.al. preserve >> >> ->msg_iter on error"). >> >> As a result, the v4.4 kernel can deliver corrupt data to the application >> >> when a corrupt UDP packet is closely followed by a valid UDP packet: the >> >> same invocation of the recvmsg() syscall can deliver the corrupt packet's >> >> UDP payload to the application with the UDP payload length and the >> >> "from IP/Port" of the valid packet. >> >> >> >> Details: >> >> >> >> For a UDP packet longer than 76 bytes (see the v5.8-rc6 kernel's >> >> include/linux/skbuff.h:3951), Linux delays the UDP checksum verification >> >> until the application invokes the syscall recvmsg(). >> >> >> >> In the recvmsg() syscall handler, while Linux is copying the UDP payload >> >> to the application's memory, it calculates the UDP checksum. If the >> >> calculated checksum doesn't match the received checksum, Linux drops the >> >> corrupt UDP packet, and then starts to process the next packet (if any), >> >> and if the next packet is valid (i.e. the checksum is correct), Linux >> >> will copy the valid UDP packet's payload to the application's receiver >> >> buffer. >> >> >> >> The bug is: before Linux starts to copy the valid UDP packet, the data >> >> structure used to track how many more bytes should be copied to the >> >> application memory is not reset to what it was when the application just >> >> entered the kernel by the syscall! Consequently, only a small portion or >> >> none of the valid packet's payload is copied to the application's >> >> receive buffer, and later when the application exits from the kernel, >> >> actually most of the application's receive buffer contains the payload >> >> of the corrupt packet while recvmsg() returns the length of the UDP >> >> payload of the valid packet. >> >> >> >> For the mainline kernel, the bug was fixed in commit 327868212381, >> >> but unluckily the bugfix is only backported to v4.9+. It turns out >> >> backporting 327868212381 to v4.4 means that some supporting patches >> >> must be backported first, so the overall changes seem too big, so the >> >> alternative is performs the csum validation earlier and drops the >> >> corrupt packets earlier. >> >> >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> >> Signed-off-by: Dexuan Cui <decui@microsoft.com> >> >> --- >> >> net/ipv4/udp.c | 3 +-- >> >> net/ipv6/udp.c | 6 ++---- >> >> 2 files changed, 3 insertions(+), 6 deletions(-) >> >> >> >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c >> >> index bb30699..49ab587 100644 >> >> --- a/net/ipv4/udp.c >> >> +++ b/net/ipv4/udp.c >> >> @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct >> >> sk_buff *skb) >> >> } >> >> } >> >> >> >> - if (rcu_access_pointer(sk->sk_filter) && >> >> - udp_lib_checksum_complete(skb)) >> >> + if (udp_lib_checksum_complete(skb)) >> >> goto csum_error; >> >> >> >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { >> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c >> >> index 73f1112..2d6703d 100644 >> >> --- a/net/ipv6/udp.c >> >> +++ b/net/ipv6/udp.c >> >> @@ -686,10 +686,8 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct >> >> sk_buff *skb) >> >> } >> >> } >> >> >> >> - if (rcu_access_pointer(sk->sk_filter)) { >> >> - if (udp_lib_checksum_complete(skb)) >> >> - goto csum_error; >> >> - } >> >> + if (udp_lib_checksum_complete(skb)) >> >> + goto csum_error; >> >> >> >> if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { >> >> UDP6_INC_STATS_BH(sock_net(sk), >> >> -- >> >> 1.8.3.1 >> > >> >+Sasha >> > >> >This patch is targeted to the linux-4.4.y branch of the stable tree. >> >> Eric, will you ack this (or have a missed a previous ack)? > >Sure, although I have already a Signed-off-by: tag on this one, since >I wrote this simpler fix for stable. > >If needed : >Acked-by: Eric Dumazet <edumazet@google.com> Ah, I see. Queued up, thanks!
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index bb30699..49ab587 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -1589,8 +1589,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) } } - if (rcu_access_pointer(sk->sk_filter) && - udp_lib_checksum_complete(skb)) + if (udp_lib_checksum_complete(skb)) goto csum_error; if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 73f1112..2d6703d 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -686,10 +686,8 @@ int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) } } - if (rcu_access_pointer(sk->sk_filter)) { - if (udp_lib_checksum_complete(skb)) - goto csum_error; - } + if (udp_lib_checksum_complete(skb)) + goto csum_error; if (sk_rcvqueues_full(sk, sk->sk_rcvbuf)) { UDP6_INC_STATS_BH(sock_net(sk),
The v4.4 stable kernel lacks this bugfix: commit 327868212381 ("make skb_copy_datagram_msg() et.al. preserve ->msg_iter on error"). As a result, the v4.4 kernel can deliver corrupt data to the application when a corrupt UDP packet is closely followed by a valid UDP packet: the same invocation of the recvmsg() syscall can deliver the corrupt packet's UDP payload to the application with the UDP payload length and the "from IP/Port" of the valid packet. Details: For a UDP packet longer than 76 bytes (see the v5.8-rc6 kernel's include/linux/skbuff.h:3951), Linux delays the UDP checksum verification until the application invokes the syscall recvmsg(). In the recvmsg() syscall handler, while Linux is copying the UDP payload to the application's memory, it calculates the UDP checksum. If the calculated checksum doesn't match the received checksum, Linux drops the corrupt UDP packet, and then starts to process the next packet (if any), and if the next packet is valid (i.e. the checksum is correct), Linux will copy the valid UDP packet's payload to the application's receiver buffer. The bug is: before Linux starts to copy the valid UDP packet, the data structure used to track how many more bytes should be copied to the application memory is not reset to what it was when the application just entered the kernel by the syscall! Consequently, only a small portion or none of the valid packet's payload is copied to the application's receive buffer, and later when the application exits from the kernel, actually most of the application's receive buffer contains the payload of the corrupt packet while recvmsg() returns the length of the UDP payload of the valid packet. For the mainline kernel, the bug was fixed in commit 327868212381, but unluckily the bugfix is only backported to v4.9+. It turns out backporting 327868212381 to v4.4 means that some supporting patches must be backported first, so the overall changes seem too big, so the alternative is performs the csum validation earlier and drops the corrupt packets earlier. Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Dexuan Cui <decui@microsoft.com> --- net/ipv4/udp.c | 3 +-- net/ipv6/udp.c | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-)