* Re: [PATCH net] net: ethernet: mediatek: avoid potential invalid memory access
2017-07-20 6:52 [PATCH net] net: ethernet: mediatek: avoid potential invalid memory access sean.wang
@ 2017-07-20 7:04 ` John Crispin
2017-07-22 11:02 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: John Crispin @ 2017-07-20 7:04 UTC (permalink / raw)
To: sean.wang, davem; +Cc: netdev, nbd, linux-mediatek, linux-kernel
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)))
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: ethernet: mediatek: avoid potential invalid memory access
2017-07-20 6:52 [PATCH net] net: ethernet: mediatek: avoid potential invalid memory access sean.wang
2017-07-20 7:04 ` John Crispin
@ 2017-07-22 11:02 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2017-07-22 11:02 UTC (permalink / raw)
To: sean.wang
Cc: kbuild-all, john, davem, nbd, netdev, linux-kernel,
linux-mediatek, Sean Wang
[-- Attachment #1: Type: text/plain, Size: 4691 bytes --]
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
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63414 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread