[bug report] mt76: mt7915: add debugfs knob for RF registers read/write

Dan Carpenter dan.carpenter at oracle.com
Thu May 19 00:30:34 PDT 2022


Hello Shayne Chen,

The patch 0a17329ae9c1: "mt76: mt7915: add debugfs knob for RF
registers read/write" from Apr 18, 2022, leads to the following
Smatch static checker warning:

	drivers/net/wireless/mediatek/mt76/mt7915/mcu.c:3757 mt7915_mcu_rf_regval()
	error: potentially dereferencing uninitialized 'skb'.

drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
    3734 int mt7915_mcu_rf_regval(struct mt7915_dev *dev, u32 regidx, u32 *val, bool set)
    3735 {
    3736         struct {
    3737                 __le32 idx;
    3738                 __le32 ofs;
    3739                 __le32 data;
    3740         } __packed req = {
    3741                 .idx = cpu_to_le32(u32_get_bits(regidx, GENMASK(31, 28))),
    3742                 .ofs = cpu_to_le32(u32_get_bits(regidx, GENMASK(27, 0))),
    3743                 .data = set ? cpu_to_le32(*val) : 0,
    3744         };
    3745         struct sk_buff *skb;
    3746         int ret;
    3747 
    3748         if (set)
    3749                 return mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD(RF_REG_ACCESS),
    3750                                          &req, sizeof(req), false);
    3751 
    3752         ret = mt76_mcu_send_and_get_msg(&dev->mt76, MCU_EXT_QUERY(RF_REG_ACCESS),
    3753                                         &req, sizeof(req), true, &skb);

Presumably this has been tested and the static checker is wrong...

But how the heck is send_and_get even supposed to work?  If the device
has a ->mcu_send_msg() op then it calls send.  The mt76_mcu_send_msg()
function uses send_and_get so that means the device has a
->mcu_send_msg() op.  So it will never ever do a get() only sends...

But that can't be true!  What is going on???

I've complained about this code before.  Nobody could explain it.  Other
people had wrong answers for how it worked so I'm not the only person
confused by it.

Update:  Oh...  *Groan*.  Send does *not* rely on ->mcu_send_msg().  It
will still send if we don't have that op.  So what must be happening is
that the caller *knows* that the op is NULL.  The only way this code can
work is when ->mcu_send_msg is NULL.  :/

    3754         if (ret)
    3755                 return ret;
    3756 
--> 3757         *val = le32_to_cpu(*(__le32 *)(skb->data + 8));
    3758         dev_kfree_skb(skb);
    3759 
    3760         return 0;
    3761 }

regards,
dan carpenter



More information about the Linux-mediatek mailing list