netdev - 答复: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp

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: <7b87919a454c4c7ba3d431783069e686@baidu.com>
Date:   Tue, 21 Jul 2020 01:42:39 +0000
From:   "Li,Rongqing" <lirongqing@...du.com>
To:     Magnus Karlsson <magnus.karlsson@...il.com>
CC:     Network Development <netdev@...r.kernel.org>,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Björn Töpel <bjorn.topel@...el.com>
Subject: 答复: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx buffer for copy mode xdp



> -----邮件原件-----
> 发件人: Magnus Karlsson [mailto:magnus.karlsson@...il.com]
> 发送时间: 2020年7月20日 15:21
> 收件人: Li,Rongqing <lirongqing@...du.com>
> 抄送: Network Development <netdev@...r.kernel.org>; intel-wired-lan
> <intel-wired-lan@...ts.osuosl.org>; Karlsson, Magnus
> <magnus.karlsson@...el.com>; Björn Töpel <bjorn.topel@...el.com>
> 主题: Re: [Intel-wired-lan] [PATCH 1/2] xdp: i40e: ixgbe: ixgbevf: not flip rx
> buffer for copy mode xdp
> 
> On Fri, Jul 17, 2020 at 8:24 AM Li RongQing <lirongqing@...du.com> wrote:
> >
> > i40e/ixgbe/ixgbevf_rx_buffer_flip in copy mode xdp can lead to data
> > corruption, like the following flow:
> >
> >    1. first skb is not for xsk, and forwarded to another device
> >       or socket queue
> >    2. seconds skb is for xsk, copy data to xsk memory, and page
> >       of skb->data is released
> >    3. rx_buff is reusable since only first skb is in it, but
> >       *_rx_buffer_flip will make that page_offset is set to
> >       first skb data
> >    4. then reuse rx buffer, first skb which still is living
> >       will be corrupted.
e, but known size type */
> >         u32 id;
> > @@ -73,6 +75,7 @@ struct xdp_buff {
> >         struct xdp_rxq_info *rxq;
> >         struct xdp_txq_info *txq;
> >         u32 frame_sz; /* frame size to deduce data_hard_end/reserved
> > tailroom*/
> > +       u32 flags;
> 
> RongQing,
> 
> Sorry that I was not clear enough. Could you please submit the simple patch
> you had, the one that only tests for the memory type.
> 
> if (xdp->rxq->mem.type != MEM_TYPE_XSK_BUFF_POOL)
>       i40e_rx_buffer_flip(rx_ring, rx_buffer, size);
> 
> I do not think that adding a flags field in the xdp_mem_info to fix an Intel driver
> problem will be hugely popular. The struct is also meant to contain long lived
> information, not things that will frequently change.
> 


Thank you Magnus

My original suggestion is wrong , it should be following

if (xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL)
       i40e_rx_buffer_flip(rx_ring, rx_buffer, size);


but I feel it is not enough to only check mem.type, it must ensure that map_type is BPF_MAP_TYPE_XSKMAP ? but it is not expose. 

other maptype, like BPF_MAP_TYPE_DEVMAP,  and if mem.type is MEM_TYPE_PAGE_SHARED, not flip the rx buffer, will cause data corruption.


-Li