From: Alexander Wetzel <alexander@wetzel-home.de>
To: Dan Carpenter <error27@gmail.com>,
oe-kbuild@lists.linux.dev, linux-wireless@vger.kernel.org
Cc: lkp@intel.com, oe-kbuild-all@lists.linux.dev,
Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH] wifi: mac80211: convert PS buffering into iTXQ
Date: Mon, 7 Nov 2022 14:52:55 +0100 [thread overview]
Message-ID: <08af0517-fb5f-cfd1-e92b-18306603144f@wetzel-home.de> (raw)
In-Reply-To: <202211060817.mqDPz8T7-lkp@intel.com>
Hi,
On 07.11.22 09:00, Dan Carpenter wrote:
> Hi Alexander,
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Alexander-Wetzel/wifi-mac80211-convert-PS-buffering-into-iTXQ/20221101-100832
> base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
> patch link: https://lore.kernel.org/r/20221031211815.6666-1-alexander%40wetzel-home.de
> patch subject: [PATCH] wifi: mac80211: convert PS buffering into iTXQ
> config: openrisc-randconfig-m041-20221106
> compiler: or1k-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
>
> New smatch warnings:
> net/mac80211/tx.c:1145 ieee80211_get_txq() warn: variable dereferenced before check 'vif' (see line 1112)
vif can't be null here, the existing null check is not needed.
ieee80211_get_txq() is only used in ieee80211_queue_skb(). Which already
access sdata->vif.type and sets vif to &sdata->vif prior of calling
ieee80211_get_txq();
Would dropping the null check in line 1145 be an acceptable solution to
get rid of this warning?
I'll then would do that in the next revision (v3) of the patch and send
that out after either Johannes has reviewed v2 or serious issues are
discovered by anyone.
>
> Old smatch warnings:
> net/mac80211/tx.c:1689 invoke_tx_handlers_late() warn: variable dereferenced before check 'tx->skb' (see line 1664)
> net/mac80211/tx.c:3365 ieee80211_xmit_fast_finish() error: we previously assumed 'key' could be null (see line 3333)
>
> vim +/vif +1145 net/mac80211/tx.c
>
> 80a83cfc434b1e Michal Kazior 2016-05-19 1104 static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1105 struct ieee80211_vif *vif,
> dbef5362111647 Michal Kazior 2017-01-13 1106 struct sta_info *sta,
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1107 struct sk_buff *skb)
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1108 {
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1109 struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1110 struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1111 struct ieee80211_txq *txq = NULL;
> df0e3c1e66212b Alexander Wetzel 2022-10-31 @1112 bool is_ap_or_mesh = (vif->type == NL80211_IFTYPE_AP ||
> ^^^^^^^^^
> New unchecked dereference.
>
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1113 ieee80211_vif_is_mesh(vif));
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1114
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1115 if (unlikely(is_ap_or_mesh &&
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1116 ieee80211_is_mgmt(hdr->frame_control) &&
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1117 !ieee80211_is_bufferable_mmpdu(hdr->frame_control)))
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1118 info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1119
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1120 if (unlikely((info->flags & IEEE80211_TX_CTL_NO_PS_BUFFER) ||
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1121 info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE ||
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1122 ieee80211_has_order(hdr->frame_control) ||
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1123 ieee80211_is_probe_req(hdr->frame_control)))
> 80a83cfc434b1e Michal Kazior 2016-05-19 1124 return NULL;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1125
> cc20ff2c6b5d3e Felix Fietkau 2020-09-08 1126 if (!(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
> 50ff477a8639fa John Crispin 2019-11-25 1127 unlikely(!ieee80211_is_data_present(hdr->frame_control))) {
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1128 if ((!ieee80211_is_mgmt(hdr->frame_control) ||
> 0eeb2b674f05cc Sara Sharon 2018-09-05 1129 ieee80211_is_bufferable_mmpdu(hdr->frame_control) ||
> 0eeb2b674f05cc Sara Sharon 2018-09-05 1130 vif->type == NL80211_IFTYPE_STATION) &&
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1131 sta && sta->uploaded) {
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1132 /*
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1133 * This will be NULL if the driver didn't set the
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1134 * opt-in hardware flag.
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1135 */
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1136 txq = sta->sta.txq[IEEE80211_NUM_TIDS];
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1137 }
> adf8ed01e4fdd2 Johannes Berg 2018-08-31 1138 } else if (sta) {
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1139 u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1140
> dbef5362111647 Michal Kazior 2017-01-13 1141 if (!sta->uploaded)
> dbef5362111647 Michal Kazior 2017-01-13 1142 return NULL;
> dbef5362111647 Michal Kazior 2017-01-13 1143
> dbef5362111647 Michal Kazior 2017-01-13 1144 txq = sta->sta.txq[tid];
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 @1145 } else if (vif) {
>
> Old code checked for NULL.
>
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1146 if (is_ap_or_mesh &&
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1147 ieee80211_hw_check(&local->hw, QUEUE_CONTROL))
> df0e3c1e66212b Alexander Wetzel 2022-10-31 1148 info->hw_queue = vif->cab_queue;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1149 txq = vif->txq;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1150 }
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1151
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1152 if (!txq)
> 80a83cfc434b1e Michal Kazior 2016-05-19 1153 return NULL;
> ba8c3d6f16a1f9 Felix Fietkau 2015-03-27 1154
> 80a83cfc434b1e Michal Kazior 2016-05-19 1155 return to_txq_info(txq);
> 80a83cfc434b1e Michal Kazior 2016-05-19 1156 }
>
next prev parent reply other threads:[~2022-11-07 14:03 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-06 0:17 [PATCH] wifi: mac80211: convert PS buffering into iTXQ kernel test robot
2022-11-07 8:00 ` Dan Carpenter
2022-11-07 13:52 ` Alexander Wetzel [this message]
2022-11-07 13:59 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2022-10-31 21:18 Alexander Wetzel
2022-11-01 8:18 ` Alexander Wetzel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=08af0517-fb5f-cfd1-e92b-18306603144f@wetzel-home.de \
--to=alexander@wetzel-home.de \
--cc=error27@gmail.com \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=lkp@intel.com \
--cc=oe-kbuild-all@lists.linux.dev \
--cc=oe-kbuild@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.