netdev - Re: [PATCH 1/1] net:openvswitch: check return value of pskb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKk2TBA18TcGMUT=r-zT-kKe69nf17idojNStosXmLPPQ@mail.gmail.com>
Date: Mon, 17 Jul 2023 17:53:26 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Yuanjun Gong <ruc_gongyuanjun@....com>
Cc: Pravin B Shelar <pshelar@....org>, "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net:openvswitch: check return value of pskb_trim()

On Mon, Jul 17, 2023 at 4:50 PM Yuanjun Gong <ruc_gongyuanjun@....com> wrote:
>
> do kfree_skb() if an unexpected result is returned by pskb_tirm()
> in do_output().
>
> Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@....com>
> ---
>  net/openvswitch/actions.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index cab1e02b63e0..6b3456bdff1c 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -920,9 +920,11 @@ static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port,
>
>                 if (unlikely(cutlen > 0)) {
>                         if (skb->len - cutlen > ovs_mac_header_len(key))
> -                               pskb_trim(skb, skb->len - cutlen);
> +                               if (pskb_trim(skb, skb->len - cutlen))
> +                                       kfree_skb(skb);
>                         else
> -                               pskb_trim(skb, ovs_mac_header_len(key));
> +                               if (pskb_trim(skb, ovs_mac_header_len(key)))
> +                                       kfree_skb(skb);
>

This patch is not correct, skb will be reused later, and UAF will happen.