Re: [PATCH] ieee802154: mcr20a: read out of bounds in mcr20a_set_channel()

On Wed, Aug 29, 2018 at 07:50:51PM +0200, Xue Liu wrote:

Hello Dan,


On Wed, 29 Aug 2018 at 16:49, Dan Carpenter [off-list ref]
wrote:

quoted

The "channel" variable can be any u8 value.  We need to make sure we
don't read outside of the PLL_INT[] or PLL_FRAC[] arrays.

I think the “channel” variable can not be any u8 value. This values is
already checked before set_channel function is called.
https://github.com/torvalds/linux/blob/master/net/ieee802154/nl802154.c#L978

Oh...  Hm...  I should have reviewed more carefully.  This patch isn't
correct.  But there may still be a bug.  What Smatch is worried about is
the other call tree:

ieee802154_start_req() calls (struct ieee802154_mlme_ops)->start_req
  -> mac802154_mlme_start_req()
     -> mac802154_dev_set_page_channel()
        -> drv_set_channel() calls local->ops->set_channel(&local->hw, page, channel);
           -> mcr20a_set_channel()

So maybe we could move the check from nl802154_set_channel() to
drv_set_channel() so channel is checked on both call trees.

regards,
dan carpenter