* [PATCH] cw1200: Fix memory leak in cw1200_set_key()
@ 2022-05-26 3:30 Jianglei Nie
2022-05-26 9:02 ` kernel test robot
0 siblings, 1 reply; 4+ messages in thread
From: Jianglei Nie @ 2022-05-26 3:30 UTC (permalink / raw)
To: pizza, kvalo, davem, edumazet, kuba, pabeni
Cc: linux-wireless, netdev, linux-kernel, Jianglei Nie
When wsm_key.index > WSM_KEY_MAX_INDEX, cw1200_set_key() returns without
calling cw1200_free_key() like other wrong paths, which may lead to a
potential memory leak.
We can fix it by calling cw1200_free_key() when some error occurs.
Signed-off-by: Jianglei Nie <niejianglei2021@163.com>
---
drivers/net/wireless/st/cw1200/sta.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/st/cw1200/sta.c b/drivers/net/wireless/st/cw1200/sta.c
index 236022d4ae2a..c0097577978d 100644
--- a/drivers/net/wireless/st/cw1200/sta.c
+++ b/drivers/net/wireless/st/cw1200/sta.c
@@ -823,6 +823,7 @@ int cw1200_set_key(struct ieee80211_hw *dev, enum set_key_cmd cmd,
};
if (wsm_key.index > WSM_KEY_MAX_INDEX) {
+ cw1200_free_key(priv, idx);
ret = -EINVAL;
goto finally;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cw1200: Fix memory leak in cw1200_set_key()
2022-05-26 3:30 [PATCH] cw1200: Fix memory leak in cw1200_set_key() Jianglei Nie
@ 2022-05-26 9:02 ` kernel test robot
2022-05-27 6:27 ` Kalle Valo
0 siblings, 1 reply; 4+ messages in thread
From: kernel test robot @ 2022-05-26 9:02 UTC (permalink / raw)
To: Jianglei Nie, pizza, kvalo, davem, edumazet, kuba, pabeni
Cc: llvm, kbuild-all, linux-wireless, netdev, linux-kernel, Jianglei Nie
Hi Jianglei,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main v5.18 next-20220526]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220526/202205261656.CWDWN8nG-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3d546191ad9d7d2ad2c7928204b9de51deafa675)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1e40283730dea11a1556d589925313cdca295484
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
git checkout 1e40283730dea11a1556d589925313cdca295484
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/wireless/st/cw1200/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net/wireless/st/cw1200/sta.c:826:26: error: use of undeclared identifier 'idx'
cw1200_free_key(priv, idx);
^
1 error generated.
vim +/idx +826 drivers/net/wireless/st/cw1200/sta.c
679
680 int cw1200_set_key(struct ieee80211_hw *dev, enum set_key_cmd cmd,
681 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
682 struct ieee80211_key_conf *key)
683 {
684 int ret = -EOPNOTSUPP;
685 struct cw1200_common *priv = dev->priv;
686 struct ieee80211_key_seq seq;
687
688 mutex_lock(&priv->conf_mutex);
689
690 if (cmd == SET_KEY) {
691 u8 *peer_addr = NULL;
692 int pairwise = (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ?
693 1 : 0;
694 int idx = cw1200_alloc_key(priv);
695 struct wsm_add_key *wsm_key = &priv->keys[idx];
696
697 if (idx < 0) {
698 ret = -EINVAL;
699 goto finally;
700 }
701
702 if (sta)
703 peer_addr = sta->addr;
704
705 key->flags |= IEEE80211_KEY_FLAG_PUT_IV_SPACE |
706 IEEE80211_KEY_FLAG_RESERVE_TAILROOM;
707
708 switch (key->cipher) {
709 case WLAN_CIPHER_SUITE_WEP40:
710 case WLAN_CIPHER_SUITE_WEP104:
711 if (key->keylen > 16) {
712 cw1200_free_key(priv, idx);
713 ret = -EINVAL;
714 goto finally;
715 }
716
717 if (pairwise) {
718 wsm_key->type = WSM_KEY_TYPE_WEP_PAIRWISE;
719 memcpy(wsm_key->wep_pairwise.peer,
720 peer_addr, ETH_ALEN);
721 memcpy(wsm_key->wep_pairwise.keydata,
722 &key->key[0], key->keylen);
723 wsm_key->wep_pairwise.keylen = key->keylen;
724 } else {
725 wsm_key->type = WSM_KEY_TYPE_WEP_DEFAULT;
726 memcpy(wsm_key->wep_group.keydata,
727 &key->key[0], key->keylen);
728 wsm_key->wep_group.keylen = key->keylen;
729 wsm_key->wep_group.keyid = key->keyidx;
730 }
731 break;
732 case WLAN_CIPHER_SUITE_TKIP:
733 ieee80211_get_key_rx_seq(key, 0, &seq);
734 if (pairwise) {
735 wsm_key->type = WSM_KEY_TYPE_TKIP_PAIRWISE;
736 memcpy(wsm_key->tkip_pairwise.peer,
737 peer_addr, ETH_ALEN);
738 memcpy(wsm_key->tkip_pairwise.keydata,
739 &key->key[0], 16);
740 memcpy(wsm_key->tkip_pairwise.tx_mic_key,
741 &key->key[16], 8);
742 memcpy(wsm_key->tkip_pairwise.rx_mic_key,
743 &key->key[24], 8);
744 } else {
745 size_t mic_offset =
746 (priv->mode == NL80211_IFTYPE_AP) ?
747 16 : 24;
748 wsm_key->type = WSM_KEY_TYPE_TKIP_GROUP;
749 memcpy(wsm_key->tkip_group.keydata,
750 &key->key[0], 16);
751 memcpy(wsm_key->tkip_group.rx_mic_key,
752 &key->key[mic_offset], 8);
753
754 wsm_key->tkip_group.rx_seqnum[0] = seq.tkip.iv16 & 0xff;
755 wsm_key->tkip_group.rx_seqnum[1] = (seq.tkip.iv16 >> 8) & 0xff;
756 wsm_key->tkip_group.rx_seqnum[2] = seq.tkip.iv32 & 0xff;
757 wsm_key->tkip_group.rx_seqnum[3] = (seq.tkip.iv32 >> 8) & 0xff;
758 wsm_key->tkip_group.rx_seqnum[4] = (seq.tkip.iv32 >> 16) & 0xff;
759 wsm_key->tkip_group.rx_seqnum[5] = (seq.tkip.iv32 >> 24) & 0xff;
760 wsm_key->tkip_group.rx_seqnum[6] = 0;
761 wsm_key->tkip_group.rx_seqnum[7] = 0;
762
763 wsm_key->tkip_group.keyid = key->keyidx;
764 }
765 break;
766 case WLAN_CIPHER_SUITE_CCMP:
767 ieee80211_get_key_rx_seq(key, 0, &seq);
768 if (pairwise) {
769 wsm_key->type = WSM_KEY_TYPE_AES_PAIRWISE;
770 memcpy(wsm_key->aes_pairwise.peer,
771 peer_addr, ETH_ALEN);
772 memcpy(wsm_key->aes_pairwise.keydata,
773 &key->key[0], 16);
774 } else {
775 wsm_key->type = WSM_KEY_TYPE_AES_GROUP;
776 memcpy(wsm_key->aes_group.keydata,
777 &key->key[0], 16);
778
779 wsm_key->aes_group.rx_seqnum[0] = seq.ccmp.pn[5];
780 wsm_key->aes_group.rx_seqnum[1] = seq.ccmp.pn[4];
781 wsm_key->aes_group.rx_seqnum[2] = seq.ccmp.pn[3];
782 wsm_key->aes_group.rx_seqnum[3] = seq.ccmp.pn[2];
783 wsm_key->aes_group.rx_seqnum[4] = seq.ccmp.pn[1];
784 wsm_key->aes_group.rx_seqnum[5] = seq.ccmp.pn[0];
785 wsm_key->aes_group.rx_seqnum[6] = 0;
786 wsm_key->aes_group.rx_seqnum[7] = 0;
787 wsm_key->aes_group.keyid = key->keyidx;
788 }
789 break;
790 case WLAN_CIPHER_SUITE_SMS4:
791 if (pairwise) {
792 wsm_key->type = WSM_KEY_TYPE_WAPI_PAIRWISE;
793 memcpy(wsm_key->wapi_pairwise.peer,
794 peer_addr, ETH_ALEN);
795 memcpy(wsm_key->wapi_pairwise.keydata,
796 &key->key[0], 16);
797 memcpy(wsm_key->wapi_pairwise.mic_key,
798 &key->key[16], 16);
799 wsm_key->wapi_pairwise.keyid = key->keyidx;
800 } else {
801 wsm_key->type = WSM_KEY_TYPE_WAPI_GROUP;
802 memcpy(wsm_key->wapi_group.keydata,
803 &key->key[0], 16);
804 memcpy(wsm_key->wapi_group.mic_key,
805 &key->key[16], 16);
806 wsm_key->wapi_group.keyid = key->keyidx;
807 }
808 break;
809 default:
810 pr_warn("Unhandled key type %d\n", key->cipher);
811 cw1200_free_key(priv, idx);
812 ret = -EOPNOTSUPP;
813 goto finally;
814 }
815 ret = wsm_add_key(priv, wsm_key);
816 if (!ret)
817 key->hw_key_idx = idx;
818 else
819 cw1200_free_key(priv, idx);
820 } else if (cmd == DISABLE_KEY) {
821 struct wsm_remove_key wsm_key = {
822 .index = key->hw_key_idx,
823 };
824
825 if (wsm_key.index > WSM_KEY_MAX_INDEX) {
> 826 cw1200_free_key(priv, idx);
827 ret = -EINVAL;
828 goto finally;
829 }
830
831 cw1200_free_key(priv, wsm_key.index);
832 ret = wsm_remove_key(priv, &wsm_key);
833 } else {
834 pr_warn("Unhandled key command %d\n", cmd);
835 }
836
837 finally:
838 mutex_unlock(&priv->conf_mutex);
839 return ret;
840 }
841
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cw1200: Fix memory leak in cw1200_set_key()
2022-05-26 9:02 ` kernel test robot
@ 2022-05-27 6:27 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2022-05-27 6:27 UTC (permalink / raw)
To: kernel test robot
Cc: Jianglei Nie, pizza, davem, edumazet, kuba, pabeni, llvm,
kbuild-all, linux-wireless, netdev, linux-kernel
kernel test robot <lkp@intel.com> writes:
> Hi Jianglei,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on wireless-next/main]
> [also build test ERROR on wireless/main v5.18 next-20220526]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
> base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
> config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220526/202205261656.CWDWN8nG-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3d546191ad9d7d2ad2c7928204b9de51deafa675)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/1e40283730dea11a1556d589925313cdca295484
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
> git checkout 1e40283730dea11a1556d589925313cdca295484
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/wireless/st/cw1200/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>>> drivers/net/wireless/st/cw1200/sta.c:826:26: error: use of undeclared identifier 'idx'
> cw1200_free_key(priv, idx);
> ^
So you don't even compile your patches? That is bad.
The patches sent to linux-wireless should be properly tested. In some
trivial cases a compilation test might enough, but even then that needs
to be clearly documented with "Compile tested only" in the commit log.
I'm getting worried how frequent it has become that people submit
untested patches or that they test them using simulation tools like
syzbot, but not on real hardware.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cw1200: Fix memory leak in cw1200_set_key()
@ 2022-05-27 6:27 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2022-05-27 6:27 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]
kernel test robot <lkp@intel.com> writes:
> Hi Jianglei,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on wireless-next/main]
> [also build test ERROR on wireless/main v5.18 next-20220526]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
> base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
> config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220526/202205261656.CWDWN8nG-lkp(a)intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3d546191ad9d7d2ad2c7928204b9de51deafa675)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/intel-lab-lkp/linux/commit/1e40283730dea11a1556d589925313cdca295484
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Jianglei-Nie/cw1200-Fix-memory-leak-in-cw1200_set_key/20220526-114747
> git checkout 1e40283730dea11a1556d589925313cdca295484
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/net/wireless/st/cw1200/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>>> drivers/net/wireless/st/cw1200/sta.c:826:26: error: use of undeclared identifier 'idx'
> cw1200_free_key(priv, idx);
> ^
So you don't even compile your patches? That is bad.
The patches sent to linux-wireless should be properly tested. In some
trivial cases a compilation test might enough, but even then that needs
to be clearly documented with "Compile tested only" in the commit log.
I'm getting worried how frequent it has become that people submit
untested patches or that they test them using simulation tools like
syzbot, but not on real hardware.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-27 6:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 3:30 [PATCH] cw1200: Fix memory leak in cw1200_set_key() Jianglei Nie
2022-05-26 9:02 ` kernel test robot
2022-05-27 6:27 ` Kalle Valo
2022-05-27 6:27 ` Kalle Valo
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.