memory leak in ext4_multi_mount_protect
Hi!
I've done debugging on this issue
https://syzkaller.appspot.com/bug?id=420258a304e5d92cfef6b0097f87b42506e1db08
and I want to ask you about
proper way of fixing it. The problem was in case sbi->s_mmp_tsk hasn’t
started at the time of kthread_stop() call. In that case allocated data
won't be freed.
I wrote fix patch, but I am confused about it, because I didn't find
any kernel code like this. I don't think, that adding new members to
struct super_block is good idea, that's why I came to that decision:
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index b9693680463a..9c33e97bd5c5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5156,8 +5156,10 @@ static int ext4_fill_super(struct super_block
*sb, void *data, int silent)
failed_mount3:
flush_work(>s_error_work);
del_timer_sync(>s_err_report);
- if (sbi->s_mmp_tsk)
- kthread_stop(sbi->s_mmp_tsk);
+ if (sbi->s_mmp_tsk) {
+ if (kthread_stop(sbi->s_mmp_tsk) == -EINTR)
+ kfree(kthread_data(sbi->s_mmp_tsk));
+ }
failed_mount2:
rcu_read_lock();
group_desc = rcu_dereference(sbi->s_group_desc);
I look forward to hearing your perspective on this patch :)
With regards,
Pavel Skripkin
Re: [PATCH] net: mac802154: fix WARNING in ieee802154_del_device
Hi! On Mon, 2021-04-12 at 07:45 -0400, Alexander Aring wrote: > Hi, > > On Mon, 12 Apr 2021 at 06:58, Pavel Skripkin > wrote: > > > > syzbot reported WARNING in ieee802154_del_device. The problem > > was in uninitialized mutex. In case of NL802154_IFTYPE_MONITOR > > mutex won't be initialized, but ieee802154_del_device() accessing > > it. > > > > Reported-by: syzbot+bf8b5834b7ec22948...@syzkaller.appspotmail.com > > Signed-off-by: Pavel Skripkin > > --- > > net/mac802154/iface.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c > > index 1cf5ac09edcb..be8d2a02c882 100644 > > --- a/net/mac802154/iface.c > > +++ b/net/mac802154/iface.c > > @@ -599,6 +599,7 @@ ieee802154_setup_sdata(struct > > ieee802154_sub_if_data *sdata, > > > > break; > > case NL802154_IFTYPE_MONITOR: > > + mutex_init(>sec_mtx); > > sdata->dev->needs_free_netdev = true; > > sdata->dev->netdev_ops = _monitor_ops; > > wpan_dev->promiscuous_mode = true; > > yes that will fix the issue, but will let the user notify that > setting > any security setting is supported by monitors which is not the case. > There are patches around which should return -EOPNOTSUPP for > monitors. > However we might support it in future to let the kernel encrypt air > frames, but this isn't supported yet and the user should be aware > that > it isn't. > Thank you for your feedback. I am still not familiar with net internals yet :) Next time I ll try to go deeper. Thanks! > - Alex With regards, Pavel Skripkin
[PATCH] net: mac802154: fix WARNING in ieee802154_del_device
syzbot reported WARNING in ieee802154_del_device. The problem was in uninitialized mutex. In case of NL802154_IFTYPE_MONITOR mutex won't be initialized, but ieee802154_del_device() accessing it. Reported-by: syzbot+bf8b5834b7ec22948...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- net/mac802154/iface.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c index 1cf5ac09edcb..be8d2a02c882 100644 --- a/net/mac802154/iface.c +++ b/net/mac802154/iface.c @@ -599,6 +599,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata, break; case NL802154_IFTYPE_MONITOR: + mutex_init(>sec_mtx); sdata->dev->needs_free_netdev = true; sdata->dev->netdev_ops = _monitor_ops; wpan_dev->promiscuous_mode = true; -- 2.31.1
Re: [PATCH] net: fix shift-out-of-bounds in nl802154_new_interface
On Tue, 2021-04-06 at 08:21 -0400, Alexander Aring wrote:
> Hi,
>
> On Mon, 5 Apr 2021 at 15:58, Pavel Skripkin
> wrote:
> >
> > syzbot reported shift-out-of-bounds in nl802154_new_interface.
> > The problem was in signed representation of enum nl802154_iftype
> >
> > enum nl802154_iftype {
> > /* for backwards compatibility TODO */
> > NL802154_IFTYPE_UNSPEC = -1,
> > ...
> >
> > Since, enum has negative value in it, objects of this type
> > will be represented as signed integer.
> >
> > type = nla_get_u32(info->attrs[NL802154_ATTR_IFTYPE]);
> >
> > u32 will be casted to signed, which can cause negative value type.
> >
> > Reported-by: syzbot+7bf7b22759195c9a2...@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin
>
> Yes, this patch will fix the issue but we discussed that the problem
> is deeper than such a fix. The real problem is that we are using a -1
> value which doesn't fit into the u32 netlink value and it gets
> converted back and forward which we should avoid.
>
OK, thanks for feedback!
>
> - Alex
With regards,
Pavel Skripkin
[PATCH] net: fix shift-out-of-bounds in nl802154_new_interface
syzbot reported shift-out-of-bounds in nl802154_new_interface.
The problem was in signed representation of enum nl802154_iftype
enum nl802154_iftype {
/* for backwards compatibility TODO */
NL802154_IFTYPE_UNSPEC = -1,
...
Since, enum has negative value in it, objects of this type
will be represented as signed integer.
type = nla_get_u32(info->attrs[NL802154_ATTR_IFTYPE]);
u32 will be casted to signed, which can cause negative value type.
Reported-by: syzbot+7bf7b22759195c9a2...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
net/ieee802154/nl802154.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 7c5a1aa5adb4..6cce045e3d40 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -910,7 +910,7 @@ static int nl802154_new_interface(struct sk_buff *skb,
struct genl_info *info)
if (info->attrs[NL802154_ATTR_IFTYPE]) {
type = nla_get_u32(info->attrs[NL802154_ATTR_IFTYPE]);
- if (type > NL802154_IFTYPE_MAX ||
+ if (type > NL802154_IFTYPE_MAX || type < 0 ||
!(rdev->wpan_phy.supported.iftypes & BIT(type)))
return -EINVAL;
}
--
2.30.2
Re: [PATCH v2] net: mac802154: Fix general protection fault
Hi! On Sun, 2021-04-04 at 20:43 -0400, Alexander Aring wrote: > Hi, > > On Thu, 4 Mar 2021 at 10:25, Pavel Skripkin > wrote: > > > > syzbot found general protection fault in crypto_destroy_tfm()[1]. > > It was caused by wrong clean up loop in llsec_key_alloc(). > > If one of the tfm array members is in IS_ERR() range it will > > cause general protection fault in clean up function [1]. > > > > Call Trace: > > crypto_free_aead include/crypto/aead.h:191 [inline] [1] > > llsec_key_alloc net/mac802154/llsec.c:156 [inline] > > mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 > > ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 > > rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] > > nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 > > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 > > genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] > > genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 > > netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 > > genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 > > netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] > > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 > > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 > > sock_sendmsg_nosec net/socket.c:654 [inline] > > sock_sendmsg+0xcf/0x120 net/socket.c:674 > > sys_sendmsg+0x6e8/0x810 net/socket.c:2350 > > ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 > > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 > > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > Signed-off-by: Pavel Skripkin > > Reported-by: syzbot+9ec037722d2603a9f...@syzkaller.appspotmail.com > > Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 > > I am sorry, I don't know the tag "Change-Id", I was doing a whole > grep > on Documentation/ without any luck. > I forgot to check the patch with ./scripts/checkpatch.pl :( > Dumb question: What is the meaning of it? This is for gerrit code review. This is required to push changes to gerrit public mirror. I'm using it to check patches with syzbot. Change ids are useless outside gerrit, so it shouldn't be here. Btw, should I sent v2 or this is already fixed? > > - Alex With regards, Pavel Skripkin
Re: [PATCH] net: netlink: fix error check in genl_family_rcv_msg_doit
Hi! On Sat, 2021-04-03 at 18:26 +0200, Johannes Berg wrote: > On Sat, 2021-04-03 at 15:13 +0000, Pavel Skripkin wrote: > > genl_family_rcv_msg_attrs_parse() can return NULL > > pointer: > > > > if (!ops->maxattr) > > return NULL; > > > > But this condition doesn't cause an error in > > genl_family_rcv_msg_doit > > And I'm almost certain that in fact it shouldn't cause an error! > > If the family doesn't set maxattr then it doesn't want to have > generic > netlink doing the parsing, but still it should be possible to call > the > ops. Look at fs/dlm/netlink.c for example, it doesn't even have > attributes. You're breaking it with this patch. > > Also, the (NULL) pointer is not actually _used_ anywhere, so why > would > it matter? > Oh, I see now. I thought, it could cause a NULL ptr deference in some cases, because some ->doit() functions accessing info.attrs directly. Now I understand the point, sorry for my misunderstanding the situation. > johannes > With regards, Pavel Skripkin
[PATCH] net: fix NULL ptr dereference in nl802154_del_llsec_key
syzbot reported NULL ptr dereference in nl802154_del_llsec_key()[1] The problem was in case of info->attrs[NL802154_ATTR_SEC_KEY] == NULL. nla_parse_nested_deprecated()[2] doesn't check this condition before calling nla_len()[3] Call Trace: nla_len include/net/netlink.h:1148 [inline] [3] nla_parse_nested_deprecated include/net/netlink.h:1231 [inline] [2] nl802154_del_llsec_key+0x16d/0x320 net/ieee802154/nl802154.c:1595 [1] genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 Reported-by: syzbot+ac5c11d2959a8b3c4...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- net/ieee802154/nl802154.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c index 7c5a1aa5adb4..2f0a138bd5eb 100644 --- a/net/ieee802154/nl802154.c +++ b/net/ieee802154/nl802154.c @@ -1592,7 +1592,8 @@ static int nl802154_del_llsec_key(struct sk_buff *skb, struct genl_info *info) struct nlattr *attrs[NL802154_KEY_ATTR_MAX + 1]; struct ieee802154_llsec_key_id id; - if (nla_parse_nested_deprecated(attrs, NL802154_KEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_KEY], nl802154_key_policy, info->extack)) + if (!info->attrs[NL802154_ATTR_SEC_KEY] || + nla_parse_nested_deprecated(attrs, NL802154_KEY_ATTR_MAX, info->attrs[NL802154_ATTR_SEC_KEY], nl802154_key_policy, info->extack)) return -EINVAL; if (ieee802154_llsec_parse_key_id(attrs[NL802154_KEY_ATTR_ID], ) < 0) -- 2.30.2
[PATCH] net: netlink: fix error check in genl_family_rcv_msg_doit
genl_family_rcv_msg_attrs_parse() can return NULL pointer: if (!ops->maxattr) return NULL; But this condition doesn't cause an error in genl_family_rcv_msg_doit Signed-off-by: Pavel Skripkin --- net/netlink/genetlink.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 2d6fdf40df66..c06d06ead181 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -719,6 +719,8 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family, GENL_DONT_VALIDATE_STRICT); if (IS_ERR(attrbuf)) return PTR_ERR(attrbuf); + if (!attrbuf) + return -EINVAL; info.snd_seq = nlh->nlmsg_seq; info.snd_portid = NETLINK_CB(skb).portid; -- 2.30.2
[PATCH] drivers: net: fix memory leak in peak_usb_create_dev
syzbot reported memory leak in peak_usb.
The problem was in case of failure after calling
->dev_init()[2] in peak_usb_create_dev()[1]. The data
allocated int dev_init() wasn't freed, so simple
->dev_free() call fix this problem.
backtrace:
[<79d6542a>] kmalloc include/linux/slab.h:552 [inline]
[<79d6542a>] kzalloc include/linux/slab.h:682 [inline]
[<79d6542a>] pcan_usb_fd_init+0x156/0x210
drivers/net/can/usb/peak_usb/pcan_usb_fd.c:868 [2]
[<c09f9057>] peak_usb_create_dev
drivers/net/can/usb/peak_usb/pcan_usb_core.c:851 [inline] [1]
[<c09f9057>] peak_usb_probe+0x389/0x490
drivers/net/can/usb/peak_usb/pcan_usb_core.c:949
Reported-by: syzbot+91adee8d9ebb9193d...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
index 573b11559d73..28e916a04047 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c
@@ -857,7 +857,7 @@ static int peak_usb_create_dev(const struct
peak_usb_adapter *peak_usb_adapter,
if (dev->adapter->dev_set_bus) {
err = dev->adapter->dev_set_bus(dev, 0);
if (err)
- goto lbl_unregister_candev;
+ goto adap_dev_free;
}
/* get device number early */
@@ -869,6 +869,10 @@ static int peak_usb_create_dev(const struct
peak_usb_adapter *peak_usb_adapter,
return 0;
+adap_dev_free:
+ if (dev->adapter->dev_free)
+ dev->adapter->dev_free(dev);
+
lbl_unregister_candev:
unregister_candev(netdev);
--
2.30.2
[PATCH] drivers: net: fix memory leak in atusb_probe
syzbot reported memory leak in atusb_probe()[1]. The problem was in atusb_alloc_urbs(). Since urb is anchored, we need to release the reference to correctly free the urb backtrace: [] kmalloc include/linux/slab.h:559 [inline] [] usb_alloc_urb+0x66/0xe0 drivers/usb/core/urb.c:74 [] atusb_alloc_urbs drivers/net/ieee802154/atusb.c:362 [inline][2] [] atusb_probe+0x158/0x820 drivers/net/ieee802154/atusb.c:1038 [1] Reported-by: syzbot+28a246747e0a46512...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- drivers/net/ieee802154/atusb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c index 0dd0ba915ab9..23ee0b14cbfa 100644 --- a/drivers/net/ieee802154/atusb.c +++ b/drivers/net/ieee802154/atusb.c @@ -365,6 +365,7 @@ static int atusb_alloc_urbs(struct atusb *atusb, int n) return -ENOMEM; } usb_anchor_urb(urb, >idle_urbs); + usb_free_urb(urb); n--; } return 0; -- 2.30.2
Memory leak in ath9k_hif_usb_dealloc_tx_urbs()
Hi!
I did some debugging on this
https://syzkaller.appspot.com/bug?id=3ea507fb3c47426497b52bd82b8ef0dd5b6cc7ee
and, I believe, I recognized the problem. The problem appears in case of
ath9k_htc_hw_init() fail. In case of this fail all tx_buf->urb krefs will be
initialized to 1, but in free function:
static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev)
static void ath9k_hif_usb_dealloc_tx_urbs(struct hif_device_usb *hif_dev)
{
...
list_for_each_entry_safe(tx_buf, tx_buf_tmp,
_dev->tx.tx_buf, list) {
usb_get_urb(tx_buf->urb);
...
usb_free_urb(tx_buf->urb);
...
}
Krefs are incremented and then decremented, that means urbs won't be freed.
I found your patch and I can't properly understand why You added
usb_get_urb(tx_buf->urb).
Can You explain please, I believe this will help me or somebody to fix this
ussue :)
With regards,
Pavel Skripkin
Re: [PATCH] wireless/nl80211.c: fix uninitialized variable
Hi! On Tue, 2021-03-30 at 14:42 +0200, Alaa Emad wrote: > > > On Mon, 29 Mar 2021 at 20:58, Greg KH > wrote: > > On Mon, Mar 29, 2021 at 08:41:38PM +0200, Alaa Emad wrote: > > > On Mon, 29 Mar 2021 at 20:20, Greg KH > > wrote: > > > > > > > On Mon, Mar 29, 2021 at 06:30:36PM +0200, Alaa Emad wrote: > > > > > Reported-by: > > syzbot+72b99dcf4607e8c77...@syzkaller.appspotmail.com > > > > > Signed-off-by: Alaa Emad > > > > > > > > You need to provide some changelog text here, I know I can not > > take > > > > patches without that, maybe the wireless maintainer is more > > flexible :) > > > > > > > you mean explain what i did , right? > > > > Yes, explain why this change is needed. > > > > > > This change fix KMSAN uninit-value in net/wireless/nl80211.c:225 , > That because of `fixedlen` variable uninitialized. > So I initialized it by zero because the code assigned value to it > after that and doesn't depend on any stored value in it . You should add this message to the patch, not just write it to maintainer. I think, this link might be useful https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html > > > > thanks, > > > > greg k-h > > > > > > Thanks , > Alaa > -- > You received this message because you are subscribed to the Google > Groups "syzkaller" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to syzkaller+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/syzkaller/CAM1DhOjWgN_0GVBeX%2Bpf%2B9mk_ysaN9pF4agAFUNEkzhxpFR4%3Dw%40mail.gmail.com > . With regards, Pavel Skripkin
[PATCH] usb: dvb-usb: fix memory leak in dvb_usb_adapter_init
syzbot reported memory leak in dvb-usb. The problem was
in invalid error handling in dvb_usb_adapter_init().
for (n = 0; n < d->props.num_adapters; n++) {
if ((ret = dvb_usb_adapter_stream_init(adap)) ||
(ret = dvb_usb_adapter_dvb_init(adap, adapter_nrs)) ||
(ret = dvb_usb_adapter_frontend_init(adap))) {
return ret;
}
...
d->num_adapters_initialized++;
...
}
In case of error in dvb_usb_adapter_dvb_init() or
dvb_usb_adapter_dvb_init() d->num_adapters_initialized won't be
incremented, but dvb_usb_adapter_exit() relies on it:
for (n = 0; n < d->num_adapters_initialized; n++)
So, allocated objects won't be freed.
Signed-off-by: Pavel Skripkin
Reported-by: syzbot+3c2be7424cea3b932...@syzkaller.appspotmail.com
---
drivers/media/usb/dvb-usb/dvb-usb-init.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dvb-usb-init.c
b/drivers/media/usb/dvb-usb/dvb-usb-init.c
index c1a7634e27b4..adc8b287326b 100644
--- a/drivers/media/usb/dvb-usb/dvb-usb-init.c
+++ b/drivers/media/usb/dvb-usb/dvb-usb-init.c
@@ -79,11 +79,17 @@ static int dvb_usb_adapter_init(struct dvb_usb_device *d,
short *adapter_nrs)
}
}
- if ((ret = dvb_usb_adapter_stream_init(adap)) ||
- (ret = dvb_usb_adapter_dvb_init(adap, adapter_nrs)) ||
- (ret = dvb_usb_adapter_frontend_init(adap))) {
+ ret = dvb_usb_adapter_stream_init(adap);
+ if (ret)
return ret;
- }
+
+ ret = dvb_usb_adapter_dvb_init(adap, adapter_nrs);
+ if (ret)
+ goto dvb_init_err;
+
+ ret = dvb_usb_adapter_frontend_init(adap);
+ if (ret)
+ goto frontend_init_err;
/* use exclusive FE lock if there is multiple shared FEs */
if (adap->fe_adap[1].fe)
@@ -103,6 +109,12 @@ static int dvb_usb_adapter_init(struct dvb_usb_device *d,
short *adapter_nrs)
}
return 0;
+
+frontend_init_err:
+ dvb_usb_adapter_dvb_exit(adap);
+dvb_init_err:
+ dvb_usb_adapter_stream_exit(adap);
+ return ret;
}
static int dvb_usb_adapter_exit(struct dvb_usb_device *d)
--
2.30.2
Re: [PATCH] tty: fix memory leak in vc_deallocate
Hi! On Sun, 2021-03-28 at 10:45 +0200, Greg KH wrote: > On Sun, Mar 28, 2021 at 12:44:43AM +0300, Pavel Skripkin wrote: > > syzbot reported memory leak in tty/vt. > > The problem was in VT_DISALLOCATE ioctl cmd. > > After allocating unimap with PIO_UNIMAP it wasn't > > freed via VT_DISALLOCATE, but vc_cons[currcons].d was > > zeroed. > > > > Signed-off-by: Pavel Skripkin > > Reported-by: syzbot+bcc922b19ccc64240...@syzkaller.appspotmail.com > > --- > > drivers/tty/vt/vt.c | 1 + > > 1 file changed, 1 insertion(+) > > Is this patch tested by syzbot to fix the problem? > Yeah, it's tested. https://syzkaller.appspot.com/bug?id=083cb8bd8468537151a57339ae72d505bb5bded0 > thanks, > > greg k-h -- With regards, Pavel Skripkin
[PATCH] tty: fix memory leak in vc_deallocate
syzbot reported memory leak in tty/vt. The problem was in VT_DISALLOCATE ioctl cmd. After allocating unimap with PIO_UNIMAP it wasn't freed via VT_DISALLOCATE, but vc_cons[currcons].d was zeroed. Signed-off-by: Pavel Skripkin Reported-by: syzbot+bcc922b19ccc64240...@syzkaller.appspotmail.com --- drivers/tty/vt/vt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 284b07224c55..0cc360da5426 100644 --- a/drivers/tty/vt/vt.c +++ b/drivers/tty/vt/vt.c @@ -1381,6 +1381,7 @@ struct vc_data *vc_deallocate(unsigned int currcons) atomic_notifier_call_chain(_notifier_list, VT_DEALLOCATE, ); vcs_remove_sysfs(currcons); visual_deinit(vc); + con_free_unimap(vc); put_pid(vc->vt_pid); vc_uniscr_set(vc, NULL); kfree(vc->vc_screenbuf); -- 2.30.2
[PATCH v2] media: usb: fix memory leak in em28xx_dvb_init
syzbot reported memory leak in em28xx_dvb_init()[1] The problem was in wrong error handling after em28xx_alloc_urbs()[2] call. In case of error allocated urbs must be freed backtrace: [] kmalloc_array.constprop.0+0x41/0x60 include/linux/slab.h:594 [] kcalloc include/linux/slab.h:623 [inline] [] em28xx_alloc_urbs+0x102/0x550 drivers/media/usb/em28xx/em28xx-core.c:930 [2] [] em28xx_dvb_init drivers/media/usb/em28xx/em28xx-dvb.c:1517 [inline] [1] Reported-by: syzbot+889397c820fa56adf...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- drivers/media/usb/em28xx/em28xx-dvb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c index 526424279637..471bd74667e3 100644 --- a/drivers/media/usb/em28xx/em28xx-dvb.c +++ b/drivers/media/usb/em28xx/em28xx-dvb.c @@ -2010,6 +2010,7 @@ static int em28xx_dvb_init(struct em28xx *dev) return result; out_free: + em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE); kfree(dvb); dev->dvb = NULL; goto ret; -- 2.30.2
Re: [PATCH] media: usb: fix memory leak in em28xx_dvb_init
Hi! On Sat, 2021-03-27 at 11:01 +0200, Andy Shevchenko wrote: > > > On Saturday, March 27, 2021, Pavel Skripkin > wrote: > > syzbot reported memory leak in em28xx_dvb_init()[1] > > The problem was in wrong error handling after > > em28xx_alloc_urbs()[2] > > call. > > In case of error allocated urbs must be freed > > > > > > I believe you may reduce twice the below backtrace and leave only > important line(s). Ok, I'll send new patch version soon. Thank you! > > > > backtrace: > > [] kmalloc_array.constprop.0+0x41/0x60 > > include/linux/slab.h:594 > > [] kcalloc include/linux/slab.h:623 [inline] > > [] em28xx_alloc_urbs+0x102/0x550 > > drivers/media/usb/em28xx/em28xx-core.c:930 [2] > > [] em28xx_dvb_init > > drivers/media/usb/em28xx/em28xx-dvb.c:1517 [inline] [1] > > [] em28xx_dvb_init.cold+0xa3/0x2bb1 > > drivers/media/usb/em28xx/em28xx-dvb.c:1483 > > [] em28xx_init_extension+0x9b/0xe0 > > drivers/media/usb/em28xx/em28xx-core.c:1126 > > [] request_module_async+0x33/0x40 > > drivers/media/usb/em28xx/em28xx-cards.c:3406 > > [] process_one_work+0x2c9/0x600 > > kernel/workqueue.c:2275 > > [] process_scheduled_works > > kernel/workqueue.c:2337 [inline] > > [] worker_thread+0x2fb/0x5d0 > > kernel/workqueue.c:2426 > > [] kthread+0x178/0x1b0 kernel/kthread.c:292 > > [] ret_from_fork+0x1f/0x30 > > arch/x86/entry/entry_64.S:294 > > > > Reported-by: syzbot+889397c820fa56adf...@syzkaller.appspotmail.com > > Signed-off-by: Pavel Skripkin > > --- > > drivers/media/usb/em28xx/em28xx-dvb.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c > > b/drivers/media/usb/em28xx/em28xx-dvb.c > > index 526424279637..471bd74667e3 100644 > > --- a/drivers/media/usb/em28xx/em28xx-dvb.c > > +++ b/drivers/media/usb/em28xx/em28xx-dvb.c > > @@ -2010,6 +2010,7 @@ static int em28xx_dvb_init(struct em28xx > > *dev) > > return result; > > > > out_free: > > + em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE); > > kfree(dvb); > > dev->dvb = NULL; > > goto ret; > > -- > > 2.30.2 > > > > > > -- With regards, Pavel Skripkin
[PATCH] media: usb: fix memory leak in em28xx_dvb_init
syzbot reported memory leak in em28xx_dvb_init()[1] The problem was in wrong error handling after em28xx_alloc_urbs()[2] call. In case of error allocated urbs must be freed backtrace: [] kmalloc_array.constprop.0+0x41/0x60 include/linux/slab.h:594 [] kcalloc include/linux/slab.h:623 [inline] [] em28xx_alloc_urbs+0x102/0x550 drivers/media/usb/em28xx/em28xx-core.c:930 [2] [] em28xx_dvb_init drivers/media/usb/em28xx/em28xx-dvb.c:1517 [inline] [1] [] em28xx_dvb_init.cold+0xa3/0x2bb1 drivers/media/usb/em28xx/em28xx-dvb.c:1483 [] em28xx_init_extension+0x9b/0xe0 drivers/media/usb/em28xx/em28xx-core.c:1126 [] request_module_async+0x33/0x40 drivers/media/usb/em28xx/em28xx-cards.c:3406 [] process_one_work+0x2c9/0x600 kernel/workqueue.c:2275 [] process_scheduled_works kernel/workqueue.c:2337 [inline] [] worker_thread+0x2fb/0x5d0 kernel/workqueue.c:2426 [] kthread+0x178/0x1b0 kernel/kthread.c:292 [] ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 Reported-by: syzbot+889397c820fa56adf...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- drivers/media/usb/em28xx/em28xx-dvb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c index 526424279637..471bd74667e3 100644 --- a/drivers/media/usb/em28xx/em28xx-dvb.c +++ b/drivers/media/usb/em28xx/em28xx-dvb.c @@ -2010,6 +2010,7 @@ static int em28xx_dvb_init(struct em28xx *dev) return result; out_free: + em28xx_uninit_usb_xfer(dev, EM28XX_DIGITAL_MODE); kfree(dvb); dev->dvb = NULL; goto ret; -- 2.30.2
Re: [PATCH v2] media: sq905.c: fix uninitialized variable
Hi!
On Fri, 2021-03-26 at 23:02 +0200, Alaa Emad wrote:
> Reported-by: syzbot+a4e309017a5f3a24c...@syzkaller.appspotmail.com
> Signed-off-by: Alaa Emad
>
> ---
> Changes in v2:
> - Fix the error occured because of pervious fix.
> ---
> drivers/media/usb/gspca/sq905.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/gspca/sq905.c
> b/drivers/media/usb/gspca/sq905.c
> index 97799cfb832e..57206dd2e1a0 100644
> --- a/drivers/media/usb/gspca/sq905.c
> +++ b/drivers/media/usb/gspca/sq905.c
> @@ -157,8 +157,8 @@ static int sq905_ack_frame(struct gspca_dev
> *gspca_dev)
> static int
> sq905_read_data(struct gspca_dev *gspca_dev, u8 *data, int size, int
> need_lock)
> {
> - int ret;
> - int act_len;
> + int ret;
> + int act_len;
>
> gspca_dev->usb_buf[0] = '\0';
> if (need_lock)
> @@ -180,8 +180,8 @@ sq905_read_data(struct gspca_dev *gspca_dev, u8
> *data, int size, int need_lock)
> data, size, _len, SQ905_DATA_TIMEOUT);
>
> /* successful, it returns 0, otherwise negative */
> - if (ret < 0 || act_len != size) {
> - pr_err("bulk read fail (%d) len %d/%d\n", ret, act_len,
> size);
> + if (ret < 0 || act_len != size) {
> + pr_err("bulk read fail (%d) len %d/%d\n", ret, ret < 0 ?
> -1 : act_len, size);
> return -EIO;
> }
> return 0;
> --
> 2.25.1
>
I skimmed through code, and I'm not sure about Dmitry's approach,
because I think usb_submit_urb() can return some valid urb-
>actual_length and error code:
if (!wait_for_completion_timeout(, expire)) {
usb_kill_urb(urb);
retval = (ctx.status == -ENOENT ? -ETIMEDOUT :
ctx.status);
dev_dbg(>dev->dev,
"%s timed out on ep%d%s len=%u/%u\n",
current->comm,
usb_endpoint_num(>ep->desc),
usb_urb_dir_in(urb) ? "in" : "out",
urb->actual_length,
urb->transfer_buffer_length);
} else
retval = ctx.status;
...
if (actual_length)
*actual_length = urb->actual_length;
I believe, that this info might be useful.
Im not sure about it, i didn't found any examples of this log and have
no idea how to reproduce it, it's just my thoughts. Maybe, one of the
maintainers will correct me
--
With regards,
Pavel Skripkin
Re: [PATCH] drivers/media/usb/gspca/stv06xx: fix memory leak
Hi! Thanks for the review.
On Tue, 2021-03-23 at 17:13 +0100, Mauro Carvalho Chehab wrote:
> Em Sat, 27 Feb 2021 02:37:31 +0300
> Pavel Skripkin escreveu:
>
> > Syzbot reported memory leak in hdcs_probe_1x00()[1].
> > hdcs_probe_1x00() allocates memory for struct hdcs, but if
> > hdcs_init() fails in gspca_dev_probe2()
> > this memory becomes leaked.
> >
> > int gspca_dev_probe2(struct usb_interface *intf,
> > const struct usb_device_id *id,
> > const struct sd_desc *sd_desc,
> > int dev_size,
> > struct module *module)
> > {
> > ...
> >
> > ret = sd_desc->config(gspca_dev, id);
> > if (ret < 0)
> > goto out;
> > ret = sd_desc->init(gspca_dev);
> > if (ret < 0)
> > goto out;
> > ...
> > out:
> > if (gspca_dev->input_dev)
> > input_unregister_device(gspca_dev->input_dev);
> > v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
> > v4l2_device_unregister(_dev->v4l2_dev);
> > kfree(gspca_dev->usb_buf);
> > kfree(gspca_dev);
> > return ret;
> > }
> >
> > Reported-by: syzbot+e7f4c64a4248a0340...@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin
> > Change-Id: Ia198671177ee346de61780813025110c7c491d7a
> > ---
> > drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c | 12
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
> > b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
> > index 5a47dcbf1c8e..24df13b33a02 100644
> > --- a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
> > +++ b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
> > @@ -485,7 +485,7 @@ static int hdcs_init(struct sd *sd)
> > stv_bridge_init[i][1]);
> > }
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > /* sensor soft reset */
> > hdcs_reset(sd);
> > @@ -496,12 +496,12 @@ static int hdcs_init(struct sd *sd)
> > stv_sensor_init[i][1]);
> > }
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > /* Enable continuous frame capture, bit 2: stop when frame
> > complete */
> > err = stv06xx_write_sensor(sd, HDCS_REG_CONFIG(sd), BIT(3));
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > /* Set PGA sample duration
> > (was 0x7E for the STV602, but caused slow framerate with
> > HDCS-1020) */
> > @@ -512,9 +512,13 @@ static int hdcs_init(struct sd *sd)
> > err = stv06xx_write_sensor(sd, HDCS_TCTRL,
> > (HDCS_ADC_START_SIG_DUR << 5) | hdcs-
> > >psmp);
> > if (err < 0)
> > - return err;
> > + goto error;
> >
> > return hdcs_set_size(sd, hdcs->array.width, hdcs-
> > >array.height);
> > +
> > +error:
> > + kfree(hdcs);
> > + return err;
> > }
>
> This doesn't seem the right fix here, as it is not the _init function
> that allocates it. Also, when the device is disconnected, a memory
> leak
> will happen.
It won't.
static int hdcs_probe_1x00(struct sd *sd)
{
sd->sensor_priv = hdcs;
}
static void sd_disconnect(struct usb_interface *intf)
{
void *priv = sd->sensor_priv;
kfree(priv);
}
Is it correct?
>
> I suspect that the right fix would be to move this:
>
> hdcs = kmalloc(sizeof(struct hdcs), GFP_KERNEL);
> if (!hdcs)
> return -ENOMEM;
>
> To the main driver (stv06xx.c) - probably replacing it by kzalloc(),
>
I don't really understand why, because this allocation refers only to
stv06xx_hdcs, and other stv06xx sensors don't use it.
If hdcs_init() fails, we won't be able to access this pointer, because
in gspca_dev_probe2() only this code will be executed on error
condition:
v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
v4l2_device_unregister(_dev->v4l2_dev);
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
Maybe, I don't properly understand it, can You explain, please?
> and then handle the free code both both sd_probe() and sd_disconnect().
>
>
> Thanks,
--
With regards,
Pavel Skripkin
Re: [PATCH] media: sq905.c: fix uninitialized variable
Hi!
On Fri, 2021-03-26 at 08:40 +0100, Dmitry Vyukov wrote:
> On Fri, Mar 26, 2021 at 8:24 AM Pavel Skripkin
> wrote:
> >
> > Hi!
> >
> > On Fri, 2021-03-26 at 08:14 +0100, 'Dmitry Vyukov' via syzkaller
> > wrote:
> > > On Fri, Mar 26, 2021 at 8:11 AM Greg KH
> > >
> > > wrote:
> > > >
> > > > On Thu, Mar 25, 2021 at 11:22:02PM +0200, Alaa Emad wrote:
> > > > > Reported-by:
> > > > > syzbot+a4e309017a5f3a24c...@syzkaller.appspotmail.com
> > > > > Signed-off-by: Alaa Emad
> > > > > ---
> > > >
> > > > I know I do not take patches with no changelog text, but other
> > > > maintainers might be more leniant :(
> > >
> > > I wonder if it's the right fix or not.
> > > Initializing variables will, of course, silence the warning, but
> > > it's
> > > not necessarily the right fix. I suspect there is something wrong
> > > in
> > > how ret/act_len are user/checked.
> > >
> >
> > There is a problem in usb_bulk_msg() call. It could return before
> > act_len initialization, so i think, act_len should be intialized with
> > smth wrong like 0 or -1. BTW, I already send patch for that, but it
> > was
> > marked as obsoleted.
>
> If usb_bulk_msg returns before act_len initialization, it should
> signify that fact with an error code in return value or something,
> right? It does not initialize act_len only in case of errors, right?
> If so, sq905_read_data must check ret and don't use act_for any
> checks. But it does, and that's I think the bug. Or maybe usb_bulk_msg
> does not properly signify that it failed (and did not initialize
> act_len). Either way silencing the warning with pre-initializing
> act_len looks very fishy.
> For example, consider, in some contexts it's OK to transmit 0-length
> packets, I don't know if it's the case for usb_bulk_msg or not, but it
> does not affect the idea. Now, if we pre-initialize act_len to 0, we
> can falsely think that such 0-length transfer has succeeded (act_len
> == size), while it actually failed (I assume so since usb_bulk_msg
> left act_len unitialized).
You are absolutely rigth, and sq905_read_data doesn't use act_len for
checks in case of usb_bulk_msg fail. But it uses it for error printing:
if (ret < 0 || act_len != size) {
pr_err("bulk read fail (%d) len %d/%d\n", ret,
act_len, size);
return -EIO;
}
So, value like -1 can be a flag for smth went wrong internally. Or
maybe remove this error log and replace it with other, which will rely
on error code, idk how it will be better
--
With regards,
Pavel Skripkin
Re: [PATCH] media: sq905.c: fix uninitialized variable
Hi! On Fri, 2021-03-26 at 08:14 +0100, 'Dmitry Vyukov' via syzkaller wrote: > On Fri, Mar 26, 2021 at 8:11 AM Greg KH > wrote: > > > > On Thu, Mar 25, 2021 at 11:22:02PM +0200, Alaa Emad wrote: > > > Reported-by: syzbot+a4e309017a5f3a24c...@syzkaller.appspotmail.com > > > Signed-off-by: Alaa Emad > > > --- > > > > I know I do not take patches with no changelog text, but other > > maintainers might be more leniant :( > > I wonder if it's the right fix or not. > Initializing variables will, of course, silence the warning, but it's > not necessarily the right fix. I suspect there is something wrong in > how ret/act_len are user/checked. > There is a problem in usb_bulk_msg() call. It could return before act_len initialization, so i think, act_len should be intialized with smth wrong like 0 or -1. BTW, I already send patch for that, but it was marked as obsoleted. -- With regards, Pavel Skripkin
[PATCH] media: usb: fix uninit-value in sq905_read_data
sybot reported uninit value in sq905_read_data().
The problem was in the error conditions in usb_bulk_msg()
before act_len initialization.
Reported-by: syzbot+a4e309017a5f3a24c...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
drivers/media/usb/gspca/sq905.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c
index 97799cfb832e..949111070971 100644
--- a/drivers/media/usb/gspca/sq905.c
+++ b/drivers/media/usb/gspca/sq905.c
@@ -158,7 +158,7 @@ static int
sq905_read_data(struct gspca_dev *gspca_dev, u8 *data, int size, int need_lock)
{
int ret;
- int act_len;
+ int act_len = 0;
gspca_dev->usb_buf[0] = '\0';
if (need_lock)
--
2.25.1
[PATCH next 2/2] sound: usb: fix use after free in usb_audio_disconnect
The problem was in wrong "if" placement. chip->quirk_type is freed
in snd_card_free_when_closed(), but inside if statement it's accesed.
Fixes: 9799110825db ("ALSA: usb-audio: Disable USB autosuspend properly in
setup_disable_autosuspend()"
Signed-off-by: Pavel Skripkin
---
sound/usb/card.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 3fd1743513b5..b6f4c0848e66 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -907,6 +907,9 @@ static void usb_audio_disconnect(struct usb_interface *intf)
}
}
+ if (chip->quirk_type & QUIRK_SETUP_DISABLE_AUTOSUSPEND)
+ usb_enable_autosuspend(interface_to_usbdev(intf));
+
chip->num_interfaces--;
if (chip->num_interfaces <= 0) {
usb_chip[chip->index] = NULL;
@@ -915,9 +918,6 @@ static void usb_audio_disconnect(struct usb_interface *intf)
} else {
mutex_unlock(_mutex);
}
-
- if (chip->quirk_type & QUIRK_SETUP_DISABLE_AUTOSUSPEND)
- usb_enable_autosuspend(interface_to_usbdev(intf));
}
/* lock the shutdown (disconnect) task and autoresume */
--
2.25.1
[PATCH next 1/2] sound: usb: fix NULL ptr dereference in usb_audio_probe
syzbot reported null pointer dereference in usb_audio_probe.
The problem was in case, when quirk == NULL. It's not an
error condition, so quirk must be checked before dereferencing.
Call Trace:
usb_probe_interface+0x315/0x7f0 drivers/usb/core/driver.c:396
really_probe+0x291/0xe60 drivers/base/dd.c:554
driver_probe_device+0x26b/0x3d0 drivers/base/dd.c:740
__device_attach_driver+0x1d1/0x290 drivers/base/dd.c:846
bus_for_each_drv+0x15f/0x1e0 drivers/base/bus.c:431
__device_attach+0x228/0x4a0 drivers/base/dd.c:914
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
device_add+0xbdb/0x1db0 drivers/base/core.c:3242
usb_set_configuration+0x113f/0x1910 drivers/usb/core/message.c:2164
usb_generic_driver_probe+0xba/0x100 drivers/usb/core/generic.c:238
usb_probe_device+0xd9/0x2c0 drivers/usb/core/driver.c:293
really_probe+0x291/0xe60 drivers/base/dd.c:554
driver_probe_device+0x26b/0x3d0 drivers/base/dd.c:740
__device_attach_driver+0x1d1/0x290 drivers/base/dd.c:846
bus_for_each_drv+0x15f/0x1e0 drivers/base/bus.c:431
__device_attach+0x228/0x4a0 drivers/base/dd.c:914
bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
device_add+0xbdb/0x1db0 drivers/base/core.c:3242
usb_new_device.cold+0x721/0x1058 drivers/usb/core/hub.c:2555
hub_port_connect drivers/usb/core/hub.c:5223 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5363 [inline]
port_event drivers/usb/core/hub.c:5509 [inline]
hub_event+0x2357/0x4320 drivers/usb/core/hub.c:5591
process_one_work+0x98d/0x1600 kernel/workqueue.c:2275
worker_thread+0x64c/0x1120 kernel/workqueue.c:2421
kthread+0x3b1/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294
Reported-by: syzbot+719da9b149a931f51...@syzkaller.appspotmail.com
Fixes: 9799110825db ("ALSA: usb-audio: Disable USB autosuspend properly in
setup_disable_autosuspend()")
Signed-off-by: Pavel Skripkin
---
sound/usb/card.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 08c794883299..3fd1743513b5 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -830,7 +830,8 @@ static int usb_audio_probe(struct usb_interface *intf,
snd_media_device_create(chip, intf);
}
- chip->quirk_type = quirk->type;
+ if (quirk)
+ chip->quirk_type = quirk->type;
usb_chip[chip->index] = chip;
chip->intf[chip->num_interfaces] = intf;
--
2.25.1
[PATCH next 0/2] fixes for sound: usb:
This small patch series fixes 2 errors from commit 9799110825db
("ALSA: usb-audio: Disable USB autosuspend properly in
setup_disable_autosuspend()").
One of them was reported by syzbot, but second one appeared while testing fixes
for the first one.
Pavel Skripkin (2):
sound: usb: fix NULL ptr dereference in usb_audio_probe
sound: usb: fix use after free in usb_audio_disconnect
sound/usb/card.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
--
2.25.1
[PATCH v2] net: mac802154: Fix general protection fault
syzbot found general protection fault in crypto_destroy_tfm()[1]. It was caused by wrong clean up loop in llsec_key_alloc(). If one of the tfm array members is in IS_ERR() range it will cause general protection fault in clean up function [1]. Call Trace: crypto_free_aead include/crypto/aead.h:191 [inline] [1] llsec_key_alloc net/mac802154/llsec.c:156 [inline] mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 sock_sendmsg_nosec net/socket.c:654 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:674 sys_sendmsg+0x6e8/0x810 net/socket.c:2350 ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xae Signed-off-by: Pavel Skripkin Reported-by: syzbot+9ec037722d2603a9f...@syzkaller.appspotmail.com Change-Id: I29f7ac641a039096d63d1e6070bb32cb5a3beb07 --- net/mac802154/llsec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c index 585d33144c33..0ead2ced 100644 --- a/net/mac802154/llsec.c +++ b/net/mac802154/llsec.c @@ -152,7 +152,7 @@ llsec_key_alloc(const struct ieee802154_llsec_key *template) crypto_free_sync_skcipher(key->tfm0); err_tfm: for (i = 0; i < ARRAY_SIZE(key->tfm); i++) - if (key->tfm[i]) + if (!IS_ERR_OR_NULL(key->tfm[i])) crypto_free_aead(key->tfm[i]); kfree_sensitive(key); -- 2.25.1
Re: [PATCH] net: mac802154: Fix null pointer dereference
Hi, thanks for your reply! On Wed, 2021-03-03 at 21:40 -0500, Alexander Aring wrote: > Hi, > > On Wed, 3 Mar 2021 at 11:28, Pavel Skripkin > wrote: > > syzbot found general protection fault in crypto_destroy_tfm()[1]. > > It was caused by wrong clean up loop in llsec_key_alloc(). > > If one of the tfm array members won't be initialized it will cause > > NULL dereference in crypto_destroy_tfm(). > > > > Call Trace: > > crypto_free_aead include/crypto/aead.h:191 [inline] [1] > > llsec_key_alloc net/mac802154/llsec.c:156 [inline] > > mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 > > ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 > > rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] > > nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 > > genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 > > genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] > > genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 > > netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 > > genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 > > netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] > > netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 > > netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 > > sock_sendmsg_nosec net/socket.c:654 [inline] > > sock_sendmsg+0xcf/0x120 net/socket.c:674 > > sys_sendmsg+0x6e8/0x810 net/socket.c:2350 > > ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 > > __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 > > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > > > Signed-off-by: Pavel Skripkin > > Reported-by: syzbot+12cf5fbfdeba210a8...@syzkaller.appspotmail.com > > --- > > net/mac802154/llsec.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c > > index 585d33144c33..6709f186f777 100644 > > --- a/net/mac802154/llsec.c > > +++ b/net/mac802154/llsec.c > > @@ -151,7 +151,7 @@ llsec_key_alloc(const struct > > ieee802154_llsec_key *template) > > err_tfm0: > > crypto_free_sync_skcipher(key->tfm0); > > err_tfm: > > - for (i = 0; i < ARRAY_SIZE(key->tfm); i++) > > + for (; i >= 0; i--) > > if (key->tfm[i]) > > I think this need to be: > > if (!IS_ERR_OR_NULL(key->tfm[i])) > > otherwise we still run into issues for the current iterator when > key->tfm[i] is in range of IS_ERR(). Oh... I got it completly wrong, I'm sorry. If it's still not fixed, I'll send rigth patch for that. > > - Alex -- With regards, Pavel Skripkin
[PATCH] net: mac802154: Fix null pointer dereference
syzbot found general protection fault in crypto_destroy_tfm()[1]. It was caused by wrong clean up loop in llsec_key_alloc(). If one of the tfm array members won't be initialized it will cause NULL dereference in crypto_destroy_tfm(). Call Trace: crypto_free_aead include/crypto/aead.h:191 [inline] [1] llsec_key_alloc net/mac802154/llsec.c:156 [inline] mac802154_llsec_key_add+0x9e0/0xcc0 net/mac802154/llsec.c:249 ieee802154_add_llsec_key+0x56/0x80 net/mac802154/cfg.c:338 rdev_add_llsec_key net/ieee802154/rdev-ops.h:260 [inline] nl802154_add_llsec_key+0x3d3/0x560 net/ieee802154/nl802154.c:1584 genl_family_rcv_msg_doit+0x228/0x320 net/netlink/genetlink.c:739 genl_family_rcv_msg net/netlink/genetlink.c:783 [inline] genl_rcv_msg+0x328/0x580 net/netlink/genetlink.c:800 netlink_rcv_skb+0x153/0x420 net/netlink/af_netlink.c:2502 genl_rcv+0x24/0x40 net/netlink/genetlink.c:811 netlink_unicast_kernel net/netlink/af_netlink.c:1312 [inline] netlink_unicast+0x533/0x7d0 net/netlink/af_netlink.c:1338 netlink_sendmsg+0x856/0xd90 net/netlink/af_netlink.c:1927 sock_sendmsg_nosec net/socket.c:654 [inline] sock_sendmsg+0xcf/0x120 net/socket.c:674 sys_sendmsg+0x6e8/0x810 net/socket.c:2350 ___sys_sendmsg+0xf3/0x170 net/socket.c:2404 __sys_sendmsg+0xe5/0x1b0 net/socket.c:2433 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xae Signed-off-by: Pavel Skripkin Reported-by: syzbot+12cf5fbfdeba210a8...@syzkaller.appspotmail.com --- net/mac802154/llsec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mac802154/llsec.c b/net/mac802154/llsec.c index 585d33144c33..6709f186f777 100644 --- a/net/mac802154/llsec.c +++ b/net/mac802154/llsec.c @@ -151,7 +151,7 @@ llsec_key_alloc(const struct ieee802154_llsec_key *template) err_tfm0: crypto_free_sync_skcipher(key->tfm0); err_tfm: - for (i = 0; i < ARRAY_SIZE(key->tfm); i++) + for (; i >= 0; i--) if (key->tfm[i]) crypto_free_aead(key->tfm[i]); -- 2.25.1
[PATCH] usb: serial: io_edgeport: fix memory leak in edge_startup
sysbot found memory leak in edge_startup().
The problem was that when an error was received from the usb_submit_urb(),
nothing was cleaned up.
Reported-by: syzbot+59f777bdcbdd7eea5...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
drivers/usb/serial/io_edgeport.c | 26 --
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index a493670c06e6..68401adcffde 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -3003,26 +3003,32 @@ static int edge_startup(struct usb_serial *serial)
response = -ENODEV;
}
- usb_free_urb(edge_serial->interrupt_read_urb);
- kfree(edge_serial->interrupt_in_buffer);
-
- usb_free_urb(edge_serial->read_urb);
- kfree(edge_serial->bulk_in_buffer);
-
- kfree(edge_serial);
-
- return response;
+ goto error;
}
/* start interrupt read for this edgeport this interrupt will
* continue as long as the edgeport is connected */
response = usb_submit_urb(edge_serial->interrupt_read_urb,
GFP_KERNEL);
- if (response)
+ if (response) {
dev_err(ddev, "%s - Error %d submitting control urb\n",
__func__, response);
+
+ goto error;
+ }
}
return response;
+
+error:
+ usb_free_urb(edge_serial->interrupt_read_urb);
+ kfree(edge_serial->interrupt_in_buffer);
+
+ usb_free_urb(edge_serial->read_urb);
+ kfree(edge_serial->bulk_in_buffer);
+
+ kfree(edge_serial);
+
+ return response;
}
--
2.25.1
[PATCH] drivers/media/usb: fix memory leak in zr364xx_probe
syzbot reported memory leak in zr364xx_probe()[1].
The problem was in invalid error handling order.
All error conditions rigth after v4l2_ctrl_handler_init()
must call v4l2_ctrl_handler_free().
Reported-by: syzbot+efe9aefc31ae1e6f7...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
drivers/media/usb/zr364xx/zr364xx.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/usb/zr364xx/zr364xx.c
b/drivers/media/usb/zr364xx/zr364xx.c
index d29b861367ea..1ef611e08323 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -1430,7 +1430,7 @@ static int zr364xx_probe(struct usb_interface *intf,
if (hdl->error) {
err = hdl->error;
dev_err(>dev, "couldn't register control\n");
- goto unregister;
+ goto free_hdlr_and_unreg_dev;
}
/* save the init method used by this camera */
cam->method = id->driver_info;
@@ -1503,7 +1503,7 @@ static int zr364xx_probe(struct usb_interface *intf,
if (!cam->read_endpoint) {
err = -ENOMEM;
dev_err(>dev, "Could not find bulk-in endpoint\n");
- goto unregister;
+ goto free_hdlr_and_unreg_dev;
}
/* v4l */
@@ -1515,7 +1515,7 @@ static int zr364xx_probe(struct usb_interface *intf,
/* load zr364xx board specific */
err = zr364xx_board_init(cam);
if (err)
- goto unregister;
+ goto free_hdlr_and_unreg_dev;
err = v4l2_ctrl_handler_setup(hdl);
if (err)
goto board_uninit;
@@ -1533,7 +1533,7 @@ static int zr364xx_probe(struct usb_interface *intf,
err = video_register_device(>vdev, VFL_TYPE_VIDEO, -1);
if (err) {
dev_err(>dev, "video_register_device failed\n");
- goto free_handler;
+ goto board_uninit;
}
cam->v4l2_dev.release = zr364xx_release;
@@ -1541,11 +1541,10 @@ static int zr364xx_probe(struct usb_interface *intf,
video_device_node_name(>vdev));
return 0;
-free_handler:
- v4l2_ctrl_handler_free(hdl);
board_uninit:
zr364xx_board_uninit(cam);
-unregister:
+free_hdlr_and_unreg_dev:
+ v4l2_ctrl_handler_free(hdl);
v4l2_device_unregister(>v4l2_dev);
free_cam:
kfree(cam);
--
2.25.1
Re: [PATCH] net/core/skbuff.c: __netdev_alloc_skb fix when len is greater than KMALLOC_MAX_SIZE
Hi, thanks for your reply!
On Mon, 2021-03-01 at 14:09 +0100, Eric Dumazet wrote:
>
> On 2/26/21 8:11 PM, Pavel Skripkin wrote:
> > syzbot found WARNING in __alloc_pages_nodemask()[1] when order >=
> > MAX_ORDER.
> > It was caused by __netdev_alloc_skb(), which doesn't check len
> > value after adding NET_SKB_PAD.
> > Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask()
> > if size > KMALLOC_MAX_SIZE.
> >
> > static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
> > {
> > struct page *page;
> > void *ptr = NULL;
> > unsigned int order = get_order(size);
> > ...
> > page = alloc_pages_node(node, flags, order);
> > ...
> >
> > [1] WARNING in __alloc_pages_nodemask+0x5f8/0x730
> > mm/page_alloc.c:5014
> > Call Trace:
> > __alloc_pages include/linux/gfp.h:511 [inline]
> > __alloc_pages_node include/linux/gfp.h:524 [inline]
> > alloc_pages_node include/linux/gfp.h:538 [inline]
> > kmalloc_large_node+0x60/0x110 mm/slub.c:3999
> > __kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
> > __kmalloc_reserve net/core/skbuff.c:150 [inline]
> > __alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
> > __netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
> > netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
> > qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
> > qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
> > call_write_iter include/linux/fs.h:1901 [inline]
> > new_sync_write+0x426/0x650 fs/read_write.c:518
> > vfs_write+0x791/0xa30 fs/read_write.c:605
> > ksys_write+0x12d/0x250 fs/read_write.c:658
> > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >
> > Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin
> > Change-Id: I480a6d6f818a4c0a387db0cd3f230b68a7daeb16
> > ---
> > net/core/skbuff.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 785daff48030..dc28c8f7bf5f 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct
> > net_device *dev, unsigned int len,
> > if (len <= SKB_WITH_OVERHEAD(1024) ||
> > len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
> > (gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
> > + if (len > KMALLOC_MAX_SIZE)
> > + return NULL;
> > +
> > skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX,
> > NUMA_NO_NODE);
> > if (!skb)
> > goto skb_fail;
> >
>
>
> No, please fix the offender instead.
Yesterday I already send newer patch version to Alexander Lobakin,
where I added __GFP_NOWARN in qrtr_endpoint_post(). I think, You can
check it in this thread.
>
> Offender tentative fix was in
>
> commit 2a80c15812372e554474b1dba0b1d8e467af295d
> Author: Sabyrzhan Tasbolatov
> Date: Tue Feb 2 15:20:59 2021 +0600
>
> net/qrtr: restrict user-controlled length in
> qrtr_tun_write_iter()
>
This patch fixes kzalloc() call, but the warning was caused by
__netdev_alloc_skb().
>
> qrtr maintainers have to tell us what is the maximum datagram length
> they need to support.
>
>
>
With regards,
Pavel Skripkin
[PATCH v4] net/qrtr: fix __netdev_alloc_skb call
syzbot found WARNING in __alloc_pages_nodemask()[1] when order >= MAX_ORDER. It was caused by a huge length value passed from userspace to qrtr_tun_write_iter(), which tries to allocate skb. Since the value comes from the untrusted source there is no need to raise a warning in __alloc_pages_nodemask(). [1] WARNING in __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5014 Call Trace: __alloc_pages include/linux/gfp.h:511 [inline] __alloc_pages_node include/linux/gfp.h:524 [inline] alloc_pages_node include/linux/gfp.h:538 [inline] kmalloc_large_node+0x60/0x110 mm/slub.c:3999 __kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496 __kmalloc_reserve net/core/skbuff.c:150 [inline] __alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210 __netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446 netdev_alloc_skb include/linux/skbuff.h:2832 [inline] qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442 qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98 call_write_iter include/linux/fs.h:1901 [inline] new_sync_write+0x426/0x650 fs/read_write.c:518 vfs_write+0x791/0xa30 fs/read_write.c:605 ksys_write+0x12d/0x250 fs/read_write.c:658 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com Signed-off-by: Pavel Skripkin --- net/qrtr/qrtr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index b34358282f37..82d2eb8c21d1 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -439,7 +439,7 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) if (len == 0 || len & 3) return -EINVAL; - skb = netdev_alloc_skb(NULL, len); + skb = __netdev_alloc_skb(NULL, len, GFP_ATOMIC | __GFP_NOWARN); if (!skb) return -ENOMEM; -- 2.25.1
[PATCH v4] net/qrtr: fix __netdev_alloc_skb call
Re: [PATCH v3] net/core/skbuff: fix passing wrong size to __alloc_skb
> From: Pavel Skripkin
> Date: Sun, 28 Feb 2021 22:28:13 +0300
>
> > Hi, thanks for reply!
> >
> > > From: Pavel Skripkin
> > > Date: Sat, 27 Feb 2021 20:51:14 +0300
> > >
> > > Hi,
> > >
> > > > syzbot found WARNING in __alloc_pages_nodemask()[1] when order
> > > > >=
> > > > MAX_ORDER.
> > > > It was caused by __netdev_alloc_skb(), which doesn't check len
> > > > value after adding NET_SKB_PAD.
> > > > Order will be >= MAX_ORDER and passed to
> > > > __alloc_pages_nodemask()
> > > > if size > KMALLOC_MAX_SIZE.
> > > > Same happens in __napi_alloc_skb.
> > > >
> > > > static void *kmalloc_large_node(size_t size, gfp_t flags, int
> > > > node)
> > > > {
> > > > struct page *page;
> > > > void *ptr = NULL;
> > > > unsigned int order = get_order(size);
> > > > ...
> > > > page = alloc_pages_node(node, flags, order);
> > > > ...
> > > >
> > > > [1] WARNING in __alloc_pages_nodemask+0x5f8/0x730
> > > > mm/page_alloc.c:5014
> > > > Call Trace:
> > > > __alloc_pages include/linux/gfp.h:511 [inline]
> > > > __alloc_pages_node include/linux/gfp.h:524 [inline]
> > > > alloc_pages_node include/linux/gfp.h:538 [inline]
> > > > kmalloc_large_node+0x60/0x110 mm/slub.c:3999
> > > > __kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
> > > > __kmalloc_reserve net/core/skbuff.c:150 [inline]
> > > > __alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
> > > > __netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
> > > > netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
> > > > qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
> > > > qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
> > > > call_write_iter include/linux/fs.h:1901 [inline]
> > > > new_sync_write+0x426/0x650 fs/read_write.c:518
> > > > vfs_write+0x791/0xa30 fs/read_write.c:605
> > > > ksys_write+0x12d/0x250 fs/read_write.c:658
> > > > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
> > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > >
> > > Ah, by the way. Have you tried to seek for the root cause, why
> > > a request for such insanely large (at least 4 Mib) skb happens
> > > in QRTR? I don't believe it's intended to be like this.
> > > Now I feel that silencing this error with early return isn't
> > > really correct approach for this.
> >
> > Yeah, i figured it out. Syzbot provides reproducer for thig bug:
> >
> > void loop(void)
> > {
> > intptr_t res = 0;
> > memcpy((void*)0x2000, "/dev/qrtr-tun\000", 14);
> > res = syscall(__NR_openat, 0xff9cul, 0x2000ul,
> > 1ul,
> > 0);
> > if (res != -1)
> > r[0] = res;
> > memcpy((void*)0x2040, "\x02", 1);
> > syscall(__NR_write, r[0], 0x2040ul, 0x40ul);
> > }
> >
> > So, simply it writes to /dev/qrtr-tun 0x40 bytes.
> > In qrtr_tun_write_iter there is a check, that the length is smaller
> > than KMALLOC_MAX_VSIZE. Then the length is passed to
> > __netdev_alloc_skb, where it becomes more than KMALLOC_MAX_VSIZE.
>
> I've checked the logics in qrtr_tun_write_iter(). Seems like it's
> only trying to prevent kzallocs of sizes larger than the maximum
> and doesn't care about netdev_alloc_skb() at all, as it ignores
> the fact that, besides NET_SKB_PAD and NET_IP_ALIGN, skbs always
> have SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) on top of
> the "usable" size.
>
> On the other hand, skb memory overheads, kmalloc bounds etc. are
> an internal thing and all related corner cases should be handled
> inside the implementations, not the users. From this point, even
> this check for (len < KMALLOC_MAX_SIZE) is a bit bogus.
> I think in that particular case with the size coming from userspace
> (i.e. untrusted source), the allocations (both kzalloc() and
> __netdev_alloc_skb()) should be performed with __GFP_NOWARN, so
> insane values won't provoke any splats.
>
> So maybe use it as a fix for this particular warning (seems like
> it's the sole place in the entire kernel that can potentially
> request such skb allocations) and don't add any branches into
> hot *alloc_skb() at all?
Well, it seems like it's better solution for this
specific warning. Thanks for quick feedback, I'll send You
Re: [PATCH v3] net/core/skbuff: fix passing wrong size to __alloc_skb
Hi, thanks for reply!
> From: Pavel Skripkin
> Date: Sat, 27 Feb 2021 20:51:14 +0300
>
> Hi,
>
> > syzbot found WARNING in __alloc_pages_nodemask()[1] when order >=
> > MAX_ORDER.
> > It was caused by __netdev_alloc_skb(), which doesn't check len
> > value after adding NET_SKB_PAD.
> > Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask()
> > if size > KMALLOC_MAX_SIZE.
> > Same happens in __napi_alloc_skb.
> >
> > static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
> > {
> > struct page *page;
> > void *ptr = NULL;
> > unsigned int order = get_order(size);
> > ...
> > page = alloc_pages_node(node, flags, order);
> > ...
> >
> > [1] WARNING in __alloc_pages_nodemask+0x5f8/0x730
> > mm/page_alloc.c:5014
> > Call Trace:
> > __alloc_pages include/linux/gfp.h:511 [inline]
> > __alloc_pages_node include/linux/gfp.h:524 [inline]
> > alloc_pages_node include/linux/gfp.h:538 [inline]
> > kmalloc_large_node+0x60/0x110 mm/slub.c:3999
> > __kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
> > __kmalloc_reserve net/core/skbuff.c:150 [inline]
> > __alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
> > __netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
> > netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
> > qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
> > qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
> > call_write_iter include/linux/fs.h:1901 [inline]
> > new_sync_write+0x426/0x650 fs/read_write.c:518
> > vfs_write+0x791/0xa30 fs/read_write.c:605
> > ksys_write+0x12d/0x250 fs/read_write.c:658
> > do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> Ah, by the way. Have you tried to seek for the root cause, why
> a request for such insanely large (at least 4 Mib) skb happens
> in QRTR? I don't believe it's intended to be like this.
> Now I feel that silencing this error with early return isn't
> really correct approach for this.
Yeah, i figured it out. Syzbot provides reproducer for thig bug:
void loop(void)
{
intptr_t res = 0;
memcpy((void*)0x2000, "/dev/qrtr-tun\000", 14);
res = syscall(__NR_openat, 0xff9cul, 0x2000ul, 1ul,
0);
if (res != -1)
r[0] = res;
memcpy((void*)0x2040, "\x02", 1);
syscall(__NR_write, r[0], 0x2040ul, 0x40ul);
}
So, simply it writes to /dev/qrtr-tun 0x40 bytes.
In qrtr_tun_write_iter there is a check, that the length is smaller
than KMALLOC_MAX_VSIZE. Then the length is passed to
__netdev_alloc_skb, where it becomes more than KMALLOC_MAX_VSIZE.
>
> > Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin
> >
> > ---
> > Changes from v3:
> > * Removed Change-Id and extra tabs in net/core/skbuff.c
> >
> > Changes from v2:
> > * Added length check to __napi_alloc_skb
> > * Added unlikely() in checks
> >
> > Change from v1:
> > * Added length check to __netdev_alloc_skb
> > ---
> > net/core/skbuff.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 785daff48030..ec7ba8728b61 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct
> > net_device *dev, unsigned int len,
> > if (len <= SKB_WITH_OVERHEAD(1024) ||
> > len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
> > (gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
> > + if (unlikely(len > KMALLOC_MAX_SIZE))
> > + return NULL;
> > +
> > skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX,
> > NUMA_NO_NODE);
> > if (!skb)
> > goto skb_fail;
> > @@ -517,6 +520,9 @@ struct sk_buff *__napi_alloc_skb(struct
> > napi_struct *napi, unsigned int len,
> > if (len <= SKB_WITH_OVERHEAD(1024) ||
> > len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
> > (gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
> > + if (unlikely(len > KMALLOC_MAX_SIZE))
> > + return NULL;
> > +
> > skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX,
> > NUMA_NO_NODE);
> > if (!skb)
> > goto skb_fail;
> > --
> > 2.25.1
>
> Thanks,
> Al
>
With regards,
Pavel Skripkin
[PATCH v3] net/core/skbuff: fix passing wrong size to __alloc_skb
syzbot found WARNING in __alloc_pages_nodemask()[1] when order >= MAX_ORDER.
It was caused by __netdev_alloc_skb(), which doesn't check len value after
adding NET_SKB_PAD.
Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask() if size >
KMALLOC_MAX_SIZE.
Same happens in __napi_alloc_skb.
static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
{
struct page *page;
void *ptr = NULL;
unsigned int order = get_order(size);
...
page = alloc_pages_node(node, flags, order);
...
[1] WARNING in __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5014
Call Trace:
__alloc_pages include/linux/gfp.h:511 [inline]
__alloc_pages_node include/linux/gfp.h:524 [inline]
alloc_pages_node include/linux/gfp.h:538 [inline]
kmalloc_large_node+0x60/0x110 mm/slub.c:3999
__kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
__kmalloc_reserve net/core/skbuff.c:150 [inline]
__alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
__netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
call_write_iter include/linux/fs.h:1901 [inline]
new_sync_write+0x426/0x650 fs/read_write.c:518
vfs_write+0x791/0xa30 fs/read_write.c:605
ksys_write+0x12d/0x250 fs/read_write.c:658
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
---
Changes from v3:
* Removed Change-Id and extra tabs in net/core/skbuff.c
Changes from v2:
* Added length check to __napi_alloc_skb
* Added unlikely() in checks
Change from v1:
* Added length check to __netdev_alloc_skb
---
net/core/skbuff.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 785daff48030..ec7ba8728b61 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
@@ -517,6 +520,9 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
--
2.25.1
[PATCH v2] net/core/skbuff: fix passing wrong size to __alloc_skb
syzbot found WARNING in __alloc_pages_nodemask()[1] when order >= MAX_ORDER.
It was caused by __netdev_alloc_skb(), which doesn't check len value after
adding NET_SKB_PAD.
Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask() if size >
KMALLOC_MAX_SIZE.
Same happens in __napi_alloc_skb.
static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
{
struct page *page;
void *ptr = NULL;
unsigned int order = get_order(size);
...
page = alloc_pages_node(node, flags, order);
...
[1] WARNING in __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5014
Call Trace:
__alloc_pages include/linux/gfp.h:511 [inline]
__alloc_pages_node include/linux/gfp.h:524 [inline]
alloc_pages_node include/linux/gfp.h:538 [inline]
kmalloc_large_node+0x60/0x110 mm/slub.c:3999
__kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
__kmalloc_reserve net/core/skbuff.c:150 [inline]
__alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
__netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
call_write_iter include/linux/fs.h:1901 [inline]
new_sync_write+0x426/0x650 fs/read_write.c:518
vfs_write+0x791/0xa30 fs/read_write.c:605
ksys_write+0x12d/0x250 fs/read_write.c:658
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
Change-Id: I480a6d6f818a4c0a387db0cd3f230b68a7daeb16
---
net/core/skbuff.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 785daff48030..a35ba145a060 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
@@ -517,6 +520,9 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
--
2.25.1
[PATCH v2] net/core/skbuff: fix passing wrong size to __alloc_skb
syzbot found WARNING in __alloc_pages_nodemask()[1] when order >= MAX_ORDER.
It was caused by __netdev_alloc_skb(), which doesn't check len value after
adding NET_SKB_PAD.
Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask() if size >
KMALLOC_MAX_SIZE.
Same happens in __napi_alloc_skb.
static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
{
struct page *page;
void *ptr = NULL;
unsigned int order = get_order(size);
...
page = alloc_pages_node(node, flags, order);
...
[1] WARNING in __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5014
Call Trace:
__alloc_pages include/linux/gfp.h:511 [inline]
__alloc_pages_node include/linux/gfp.h:524 [inline]
alloc_pages_node include/linux/gfp.h:538 [inline]
kmalloc_large_node+0x60/0x110 mm/slub.c:3999
__kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
__kmalloc_reserve net/core/skbuff.c:150 [inline]
__alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
__netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
call_write_iter include/linux/fs.h:1901 [inline]
new_sync_write+0x426/0x650 fs/read_write.c:518
vfs_write+0x791/0xa30 fs/read_write.c:605
ksys_write+0x12d/0x250 fs/read_write.c:658
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
Change-Id: I480a6d6f818a4c0a387db0cd3f230b68a7daeb16
---
net/core/skbuff.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 785daff48030..a35ba145a060 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
@@ -517,6 +520,9 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (unlikely(len > KMALLOC_MAX_SIZE))
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
--
2.25.1
[PATCH] drivers/media/usb/gspca/stv06xx: fix memory leak
Syzbot reported memory leak in hdcs_probe_1x00()[1].
hdcs_probe_1x00() allocates memory for struct hdcs, but if hdcs_init() fails in
gspca_dev_probe2()
this memory becomes leaked.
int gspca_dev_probe2(struct usb_interface *intf,
const struct usb_device_id *id,
const struct sd_desc *sd_desc,
int dev_size,
struct module *module)
{
...
ret = sd_desc->config(gspca_dev, id);
if (ret < 0)
goto out;
ret = sd_desc->init(gspca_dev);
if (ret < 0)
goto out;
...
out:
if (gspca_dev->input_dev)
input_unregister_device(gspca_dev->input_dev);
v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
v4l2_device_unregister(_dev->v4l2_dev);
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
return ret;
}
Reported-by: syzbot+e7f4c64a4248a0340...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
Change-Id: Ia198671177ee346de61780813025110c7c491d7a
---
drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
index 5a47dcbf1c8e..24df13b33a02 100644
--- a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
+++ b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
@@ -485,7 +485,7 @@ static int hdcs_init(struct sd *sd)
stv_bridge_init[i][1]);
}
if (err < 0)
- return err;
+ goto error;
/* sensor soft reset */
hdcs_reset(sd);
@@ -496,12 +496,12 @@ static int hdcs_init(struct sd *sd)
stv_sensor_init[i][1]);
}
if (err < 0)
- return err;
+ goto error;
/* Enable continuous frame capture, bit 2: stop when frame complete */
err = stv06xx_write_sensor(sd, HDCS_REG_CONFIG(sd), BIT(3));
if (err < 0)
- return err;
+ goto error;
/* Set PGA sample duration
(was 0x7E for the STV602, but caused slow framerate with HDCS-1020) */
@@ -512,9 +512,13 @@ static int hdcs_init(struct sd *sd)
err = stv06xx_write_sensor(sd, HDCS_TCTRL,
(HDCS_ADC_START_SIG_DUR << 5) | hdcs->psmp);
if (err < 0)
- return err;
+ goto error;
return hdcs_set_size(sd, hdcs->array.width, hdcs->array.height);
+
+error:
+ kfree(hdcs);
+ return err;
}
static int hdcs_dump(struct sd *sd)
--
2.25.1
[PATCH] drivers/media/usb/gspca/stv06xx: fix memory leak
Syzbot reported memory leak in hdcs_probe_1x00()[1].
hdcs_probe_1x00() allocates memory for struct hdcs, but if hdcs_init() fails in
gspca_dev_probe2()
this memory becomes leaked.
int gspca_dev_probe2(struct usb_interface *intf,
const struct usb_device_id *id,
const struct sd_desc *sd_desc,
int dev_size,
struct module *module)
{
...
ret = sd_desc->config(gspca_dev, id);
if (ret < 0)
goto out;
ret = sd_desc->init(gspca_dev);
if (ret < 0)
goto out;
...
out:
if (gspca_dev->input_dev)
input_unregister_device(gspca_dev->input_dev);
v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler);
v4l2_device_unregister(_dev->v4l2_dev);
kfree(gspca_dev->usb_buf);
kfree(gspca_dev);
return ret;
}
Reported-by: syzbot+e7f4c64a4248a0340...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
Change-Id: Ia198671177ee346de61780813025110c7c491d7a
---
drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
index 5a47dcbf1c8e..24df13b33a02 100644
--- a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
+++ b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c
@@ -485,7 +485,7 @@ static int hdcs_init(struct sd *sd)
stv_bridge_init[i][1]);
}
if (err < 0)
- return err;
+ goto error;
/* sensor soft reset */
hdcs_reset(sd);
@@ -496,12 +496,12 @@ static int hdcs_init(struct sd *sd)
stv_sensor_init[i][1]);
}
if (err < 0)
- return err;
+ goto error;
/* Enable continuous frame capture, bit 2: stop when frame complete */
err = stv06xx_write_sensor(sd, HDCS_REG_CONFIG(sd), BIT(3));
if (err < 0)
- return err;
+ goto error;
/* Set PGA sample duration
(was 0x7E for the STV602, but caused slow framerate with HDCS-1020) */
@@ -512,9 +512,13 @@ static int hdcs_init(struct sd *sd)
err = stv06xx_write_sensor(sd, HDCS_TCTRL,
(HDCS_ADC_START_SIG_DUR << 5) | hdcs->psmp);
if (err < 0)
- return err;
+ goto error;
return hdcs_set_size(sd, hdcs->array.width, hdcs->array.height);
+
+error:
+ kfree(hdcs);
+ return err;
}
static int hdcs_dump(struct sd *sd)
--
2.25.1
[PATCH] net/core/skbuff.c: __netdev_alloc_skb fix when len is greater than KMALLOC_MAX_SIZE
syzbot found WARNING in __alloc_pages_nodemask()[1] when order >= MAX_ORDER.
It was caused by __netdev_alloc_skb(), which doesn't check len value after
adding NET_SKB_PAD.
Order will be >= MAX_ORDER and passed to __alloc_pages_nodemask() if size >
KMALLOC_MAX_SIZE.
static void *kmalloc_large_node(size_t size, gfp_t flags, int node)
{
struct page *page;
void *ptr = NULL;
unsigned int order = get_order(size);
...
page = alloc_pages_node(node, flags, order);
...
[1] WARNING in __alloc_pages_nodemask+0x5f8/0x730 mm/page_alloc.c:5014
Call Trace:
__alloc_pages include/linux/gfp.h:511 [inline]
__alloc_pages_node include/linux/gfp.h:524 [inline]
alloc_pages_node include/linux/gfp.h:538 [inline]
kmalloc_large_node+0x60/0x110 mm/slub.c:3999
__kmalloc_node_track_caller+0x319/0x3f0 mm/slub.c:4496
__kmalloc_reserve net/core/skbuff.c:150 [inline]
__alloc_skb+0x4e4/0x5a0 net/core/skbuff.c:210
__netdev_alloc_skb+0x70/0x400 net/core/skbuff.c:446
netdev_alloc_skb include/linux/skbuff.h:2832 [inline]
qrtr_endpoint_post+0x84/0x11b0 net/qrtr/qrtr.c:442
qrtr_tun_write_iter+0x11f/0x1a0 net/qrtr/tun.c:98
call_write_iter include/linux/fs.h:1901 [inline]
new_sync_write+0x426/0x650 fs/read_write.c:518
vfs_write+0x791/0xa30 fs/read_write.c:605
ksys_write+0x12d/0x250 fs/read_write.c:658
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reported-by: syzbot+80dccaee7c6630fa9...@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin
Change-Id: I480a6d6f818a4c0a387db0cd3f230b68a7daeb16
---
net/core/skbuff.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 785daff48030..dc28c8f7bf5f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -443,6 +443,9 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
unsigned int len,
if (len <= SKB_WITH_OVERHEAD(1024) ||
len > SKB_WITH_OVERHEAD(PAGE_SIZE) ||
(gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+ if (len > KMALLOC_MAX_SIZE)
+ return NULL;
+
skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
if (!skb)
goto skb_fail;
--
2.25.1
