Re: [PATCH v6 02/17] nl80211: Migrate to current netlink key message format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Sep 15, 2019 at 10:08:22PM +0200, Alexander Wetzel wrote:
@@ -3045,26 +3046,31 @@ static int wpa_driver_nl80211_set_key(const char *ifname, struct i802_bss *bss,

+	key_msg = nlmsg_alloc();
...

+	if (nla_put_u8(key_msg, NL80211_KEY_IDX, key_idx) ||
+	    nla_put_nested(msg, NL80211_ATTR_KEY, key_msg))
  		goto fail;
...

+	key_msg = nlmsg_alloc();

This seems to leak memory (that nla_put_nested() used key_msg, but did
not free it). And also leave in key information in heap.

Thanks for pointing that out...

It kind of makes sense: I was simply assuming that nla_put_nested() would fold the information into the existing netlink message and never tough to cross check that. But when it's copying the information we have a memory leak...

So I've also now added
	nlmsg_free(key_msg);
after each call to nla_put_nested() in my tree.


+	if (nla_put_nested(msg, NL80211_ATTR_KEY, key_msg))
+		goto fail;
+
  	ret = send_and_recv_msgs(drv, msg, NULL, NULL);

Same here.

+fail2:
+	nl80211_nlmsg_clear(key_msg);
+	nlmsg_free(key_msg);

These need to be done in the success cases as well.

No need to send this patch again because of this, though, since I've
already addressed that in my work version.


Ok.

But as always when sending a patch out I already found another issue:

handle_extended_key_id() for hostapd is kind of stupid in the current patchset. By moving it some lines we can also fold the TKIP check into it. I've mostly done that - more tests pending - but then I also noticed that handle_extended_key() should be ok to check less and want to look into that, too. (Should be trivial, so I probably can do that today.)

I'm also thinking about to dropping all the "& 0x03" in src/ap/wpa_auth.c. The one I added for keyidx_active should be "& 0x01" but since we do that on trusted internal variables which already have the correct values there should be no need to do that at all...

Do you maybe prefer just a diff with the changes compared to V6?
I could either act like V6 has been merged or even just send you one patch with the accumulated fixes.


Alexander

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux