Messages in this thread | | | Date | Wed, 25 Jan 2023 13:04:10 +0900 | Subject | Re: [Intel-wired-lan] [PATCH RESEND] igbvf: Fix rx_buffer_len | From | Akihiko Odaki <> |
| |
On 2023/01/24 19:49, Paul Menzel wrote: > Dear Akihiko, > > > Thank you for your patch. > > Am 24.01.23 um 05:39 schrieb Akihiko Odaki: > > Maybe improve the commit message summary to be more specific: > > igbvf: Align rx_buffer_len to fix memory corrption > >> When rx_buffer_len is not aligned by 1024, igbvf sets the aligned size >> to SRRCTL while the buffer is allocated with the unaligned size. This >> allows the device to write more data than rx_buffer_len, resulting in >> memory corruption. Align rx_buffer_len itself so that the buffer will >> be allocated with the aligned size. >> >> The condition to split RX packet header, which uses rx_buffer_len, is >> also modified so that it doesn't change the behavior for the same >> actual (unaligned) packet size. Actually the new condition is not >> identical with the old one as it will no longer request splitting when >> the actual packet size is exactly 2048, but that should be negligible. > > Is there an easy way to reproduce it? > > > Kind regards, > > Paul > >
I withdraw this patch. While igbvf sets a value greater than the actual buffer size to SRRCTL.BSIZEPKT, such a long packet should be dropped according to VMOLR.RLPML.
Regards, Akihiko Odaki
>> Fixes: d4e0fe01a38a ("igbvf: add new driver to support 82576 virtual >> functions") >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> >> --- >> drivers/net/ethernet/intel/igbvf/netdev.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c >> b/drivers/net/ethernet/intel/igbvf/netdev.c >> index 3a32809510fc..b6bca78198fa 100644 >> --- a/drivers/net/ethernet/intel/igbvf/netdev.c >> +++ b/drivers/net/ethernet/intel/igbvf/netdev.c >> @@ -1341,10 +1341,9 @@ static void igbvf_setup_srrctl(struct >> igbvf_adapter *adapter) >> srrctl |= E1000_SRRCTL_DROP_EN; >> /* Setup buffer sizes */ >> - srrctl |= ALIGN(adapter->rx_buffer_len, 1024) >> >> - E1000_SRRCTL_BSIZEPKT_SHIFT; >> + srrctl |= adapter->rx_buffer_len >> E1000_SRRCTL_BSIZEPKT_SHIFT; >> - if (adapter->rx_buffer_len < 2048) { >> + if (adapter->rx_buffer_len <= 2048) { >> adapter->rx_ps_hdr_size = 0; >> srrctl |= E1000_SRRCTL_DESCTYPE_ADV_ONEBUF; >> } else { >> @@ -1625,7 +1624,7 @@ static int igbvf_sw_init(struct igbvf_adapter >> *adapter) >> struct net_device *netdev = adapter->netdev; >> s32 rc; >> - adapter->rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN + ETH_FCS_LEN; >> + adapter->rx_buffer_len = ALIGN(ETH_FRAME_LEN + VLAN_HLEN + >> ETH_FCS_LEN, 1024); >> adapter->rx_ps_hdr_size = 0; >> adapter->max_frame_size = netdev->mtu + ETH_HLEN + ETH_FCS_LEN; >> adapter->min_frame_size = ETH_ZLEN + ETH_FCS_LEN; >> @@ -2429,6 +2428,8 @@ static int igbvf_change_mtu(struct net_device >> *netdev, int new_mtu) >> adapter->rx_buffer_len = ETH_FRAME_LEN + VLAN_HLEN + >> ETH_FCS_LEN; >> + adapter->rx_buffer_len = ALIGN(adapter->rx_buffer_len, 1024); >> + >> netdev_dbg(netdev, "changing MTU from %d to %d\n", >> netdev->mtu, new_mtu); >> netdev->mtu = new_mtu;
| |