Message ID | 3a9d3ee4bcfd47487c5dbac4c658f10e0233b723.1500532842.git.sean.wang@mediatek.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On 20/07/17 08:52, sean.wang@mediatek.com wrote: > From: Sean Wang <sean.wang@mediatek.com> > > Potential dangerous invalid memory might be accessed if invalid mac value > reflected from the forward port field in rxd4 caused by possible potential > hardware defects. So added a simple sanity checker to avoid the kind of > situation happening. > > Signed-off-by: Sean Wang <sean.wang@mediatek.com> Thanks, i ran into the same problem last week and was going to send a fix shortly. Acked-by: John Crispin <john@phrozen.org> > --- > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > index c1dc08c..8175433 100644 > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c > @@ -999,6 +999,12 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget, > RX_DMA_FPORT_MASK; > mac--; > > + if (unlikely(mac < 0 || mac >= MTK_MAC_COUNT || > + !eth->netdev[mac])) { > + netdev->stats.rx_dropped++; > + goto release_desc; > + } > + > netdev = eth->netdev[mac]; > > if (unlikely(test_bit(MTK_RESETTING, ð->state)))
Hi Sean, [auto build test WARNING on net/master] url: https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/net-ethernet-mediatek-avoid-potential-invalid-memory-access/20170722-155541 config: arm-allmodconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm Note: it may well be a FALSE warning. FWIW you are at least aware of it now. http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings All warnings (new ones prefixed by >>): drivers/net//ethernet/mediatek/mtk_eth_soc.c: In function 'mtk_napi_rx': >> drivers/net//ethernet/mediatek/mtk_eth_soc.c:952:28: warning: 'netdev' may be used uninitialized in this function [-Wmaybe-uninitialized] netdev->stats.rx_dropped++; ~~~~~~~~~~~~~~~~~~~~~~~~^~ drivers/net//ethernet/mediatek/mtk_eth_soc.c:928:22: note: 'netdev' was declared here struct net_device *netdev; ^~~~~~ vim +/netdev +952 drivers/net//ethernet/mediatek/mtk_eth_soc.c 916 917 static int mtk_poll_rx(struct napi_struct *napi, int budget, 918 struct mtk_eth *eth) 919 { 920 struct mtk_rx_ring *ring; 921 int idx; 922 struct sk_buff *skb; 923 u8 *data, *new_data; 924 struct mtk_rx_dma *rxd, trxd; 925 int done = 0; 926 927 while (done < budget) { 928 struct net_device *netdev; 929 unsigned int pktlen; 930 dma_addr_t dma_addr; 931 int mac = 0; 932 933 ring = mtk_get_rx_ring(eth); 934 if (unlikely(!ring)) 935 goto rx_done; 936 937 idx = NEXT_RX_DESP_IDX(ring->calc_idx, ring->dma_size); 938 rxd = &ring->dma[idx]; 939 data = ring->data[idx]; 940 941 mtk_rx_get_desc(&trxd, rxd); 942 if (!(trxd.rxd2 & RX_DMA_DONE)) 943 break; 944 945 /* find out which mac the packet come from. values start at 1 */ 946 mac = (trxd.rxd4 >> RX_DMA_FPORT_SHIFT) & 947 RX_DMA_FPORT_MASK; 948 mac--; 949 950 if (unlikely(mac < 0 || mac >= MTK_MAC_COUNT || 951 !eth->netdev[mac])) { > 952 netdev->stats.rx_dropped++; 953 goto release_desc; 954 } 955 956 netdev = eth->netdev[mac]; 957 958 if (unlikely(test_bit(MTK_RESETTING, ð->state))) 959 goto release_desc; 960 961 /* alloc new buffer */ 962 new_data = napi_alloc_frag(ring->frag_size); 963 if (unlikely(!new_data)) { 964 netdev->stats.rx_dropped++; 965 goto release_desc; 966 } 967 dma_addr = dma_map_single(eth->dev, 968 new_data + NET_SKB_PAD, 969 ring->buf_size, 970 DMA_FROM_DEVICE); 971 if (unlikely(dma_mapping_error(eth->dev, dma_addr))) { 972 skb_free_frag(new_data); 973 netdev->stats.rx_dropped++; 974 goto release_desc; 975 } 976 977 /* receive data */ 978 skb = build_skb(data, ring->frag_size); 979 if (unlikely(!skb)) { 980 skb_free_frag(new_data); 981 netdev->stats.rx_dropped++; 982 goto release_desc; 983 } 984 skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); 985 986 dma_unmap_single(eth->dev, trxd.rxd1, 987 ring->buf_size, DMA_FROM_DEVICE); 988 pktlen = RX_DMA_GET_PLEN0(trxd.rxd2); 989 skb->dev = netdev; 990 skb_put(skb, pktlen); 991 if (trxd.rxd4 & RX_DMA_L4_VALID) 992 skb->ip_summed = CHECKSUM_UNNECESSARY; 993 else 994 skb_checksum_none_assert(skb); 995 skb->protocol = eth_type_trans(skb, netdev); 996 997 if (netdev->features & NETIF_F_HW_VLAN_CTAG_RX && 998 RX_DMA_VID(trxd.rxd3)) 999 __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), 1000 RX_DMA_VID(trxd.rxd3)); 1001 skb_record_rx_queue(skb, 0); 1002 napi_gro_receive(napi, skb); 1003 1004 ring->data[idx] = new_data; 1005 rxd->rxd1 = (unsigned int)dma_addr; 1006 1007 release_desc: 1008 rxd->rxd2 = RX_DMA_PLEN0(ring->buf_size); 1009 1010 ring->calc_idx = idx; 1011 1012 done++; 1013 } 1014 1015 rx_done: 1016 if (done) { 1017 /* make sure that all changes to the dma ring are flushed before 1018 * we continue 1019 */ 1020 wmb(); 1021 mtk_update_rx_cpu_idx(eth); 1022 } 1023 1024 return done; 1025 } 1026 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index c1dc08c..8175433 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -999,6 +999,12 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget, RX_DMA_FPORT_MASK; mac--; + if (unlikely(mac < 0 || mac >= MTK_MAC_COUNT || + !eth->netdev[mac])) { + netdev->stats.rx_dropped++; + goto release_desc; + } + netdev = eth->netdev[mac]; if (unlikely(test_bit(MTK_RESETTING, ð->state)))