Hi Hui,

>       u8                    *data_ind
>  )
>  {
> -     struct ieee802154_hdr hdr;
> +     struct ieee802154_hdr hdr = { };
>       int msdulen;
>       int hlen;
> -     u8 mpdulinkquality = data_ind[23];
> +     u8 mpdulinkquality;
>       struct sk_buff *skb;
>       struct ca8210_priv *priv = hw->priv;
>  
> +     if (len < 30)
> +             return -EINVAL;
> +     mpdulinkquality = data_ind[23];
> +
>       /* Allocate mtu size buffer for every rx packet */
>       skb = dev_alloc_skb(IEEE802154_MTU + sizeof(hdr));
>       if (!skb)
> @@ -1770,7 +1778,7 @@ static int ca8210_skb_rx(
>       skb_reserve(skb, sizeof(hdr));
>  
>       msdulen = data_ind[22]; /* msdu_length */
> -     if (msdulen > IEEE802154_MTU) {
> +     if (msdulen > IEEE802154_MTU || len < 30 + msdulen) {
>               dev_err(
>                       &priv->spi->dev,
>                       "received erroneously large msdu length!\n"
> @@ -1787,6 +1795,10 @@ static int ca8210_skb_rx(
>       hdr.sec.level = data_ind[29 + msdulen];
>       dev_dbg(&priv->spi->dev, "security level: %#03x\n", hdr.sec.level);
>       if (hdr.sec.level > 0) {
> +             if (len < 40 + msdulen) {

Can we have only one place where we make all the checks?

> +                     kfree_skb(skb);
> +                     return -EINVAL;
> +             }
>               hdr.sec.key_id_mode = data_ind[30 + msdulen];
>               memcpy(&hdr.sec.extended_src, &data_ind[31 + msdulen], 8);
>               hdr.sec.key_id = data_ind[39 + msdulen];
> @@ -1801,6 +1813,7 @@ static int ca8210_skb_rx(
>       hdr.dest.pan_id = cpu_to_le16(get_unaligned_le16(&data_ind[12]));
>       dev_dbg(&priv->spi->dev, "dstPanId: %#06x\n", hdr.dest.pan_id);
>       memcpy(&hdr.dest.extended_addr, &data_ind[14], 8);
> +     hdr.seq = data_ind[24];

Where does this come from?

>       /* Fill in FC implicitly */
>       hdr.fc.type = 1; /* Data frame */
> @@ -2028,11 +2041,14 @@ static int ca8210_xmit_async(struct ieee802154_hw 
> *hw, struct sk_buff *skb)
>  static int ca8210_get_ed(struct ieee802154_hw *hw, u8 *level)
>  {
>       u8 lenvar;
> +     u8 buf[sizeof(((struct hwme_get_confirm_pset *)0)->hw_attribute_value)] 
> = { 0 };

I'm not a fan of this. It is totally unreadable.

> +     u8 status;
>       struct ca8210_priv *priv = hw->priv;
>  
> -     return link_to_linux_err(
> -             hwme_get_request_sync(HWME_EDVALUE, &lenvar, level, priv->spi)
> -     );
> +     status = hwme_get_request_sync(HWME_EDVALUE, &lenvar, buf, priv->spi);
> +     if (status == IEEE802154_SUCCESS)
> +             *level = buf[0];
> +     return link_to_linux_err(status);
>  }
>  
>  /**

Reply via email to