Re: [PATCH] drivers/mt: restrict num_slots in input_mt_init_slots()

[Sabyrzhan Tasbolatov]

On Tue,  2 Feb 2021 18:08:07 +0600, Sabyrzhan Tasbolatov wrote:
> syzbot found WARNING in input_mt_init_slots [1] when
> struct_size(mt, slots, num_slots)=0x40006 where num_slots=0x10001,
> which exceeds KMALLOC_MAX_SIZE (0x4) and causes
> order >= MAX_ORDER condition.
> 
> [1]
> Call Trace:
>  alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
>  alloc_pages include/linux/gfp.h:547 [inline]
>  kmalloc_order+0x2e/0xb0 mm/slab_common.c:837
>  kmalloc_order_trace+0x14/0x120 mm/slab_common.c:853
>  kmalloc include/linux/slab.h:557 [inline]
>  kzalloc include/linux/slab.h:682 [inline]
>  input_mt_init_slots drivers/input/input-mt.c:49 [inline]
> 
> Reported-by: syzbot+0122fa359a6969439...@syzkaller.appspotmail.com
> Signed-off-by: Sabyrzhan Tasbolatov 
> ---
>  drivers/input/input-mt.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index 44fe6f2f063c..e542f45a45ab 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -40,13 +40,18 @@ int input_mt_init_slots(struct input_dev *dev, unsigned 
> int num_slots,
>  {
>   struct input_mt *mt = dev->mt;
>   int i;
> + size_t mt_size = 0;
>  
>   if (!num_slots)
>   return 0;
>   if (mt)
>   return mt->num_slots != num_slots ? -EINVAL : 0;
>  
> - mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
> + mt_size = struct_size(mt, slots, num_slots);
> + if (mt_size > KMALLOC_MAX_SIZE)
> + return -ENOMEM;
> +
> + mt = kzalloc(mt_size, GFP_KERNEL);
>   if (!mt)
>   goto err_mem;
>  
> -- 

Following-up. I've also just found out that in this function, there is another
allocation with num_slots length:

int input_mt_init_slots(..)
{
..
if (flags & INPUT_MT_TRACK) {
unsigned int n2 = num_slots * num_slots;
mt->red = kcalloc(n2, sizeof(*mt->red), GFP_KERNEL);
..

I've checked HID vendors' xrefs for input_mt_init_slots(), most of them
pass >= 5 value to num_slots parameter. So either we should choose some
optimal limit of num_slots or just restrict it with big KMALLOC_MAX_SIZE.

Comments?

Re: [PATCH] drivers/hid: fix for the big hid report length

[Sabyrzhan Tasbolatov]

On Thu, 25 Feb 2021 10:59:14 -0500, Alan Stern wrote:
> Won't this cause silent errors?

Agree. But there are already such as cases like in:

// net/bluetooth/hidp/core.c
static void hidp_process_report(..)
{
..
if (len > HID_MAX_BUFFER_SIZE)
len = HID_MAX_BUFFER_SIZE;
..

// drivers/hid/hid-core.c
int hid_report_raw_event(..)
{
..
rsize = hid_compute_report_size(report);

if (report_enum->numbered && rsize >= HID_MAX_BUFFER_SIZE)
rsize = HID_MAX_BUFFER_SIZE - 1;
else if (rsize > HID_MAX_BUFFER_SIZE)
rsize = HID_MAX_BUFFER_SIZE;
..

// drivers/staging/greybus/hid.c
static int gb_hid_start(..)
{
..
if (bufsize > HID_MAX_BUFFER_SIZE)
bufsize = HID_MAX_BUFFER_SIZE;
..

> How about instead just rejecting any device which includes a report 
> whose length is too big (along with an line in the system log explaining 
> what's wrong)?

Not everywhere, but there are already such as logs when > HID_MAX_BUFFER_SIZE

// drivers/hid/hidraw.c
static ssize_t hidraw_send_report(..)
{
..
if (count > HID_MAX_BUFFER_SIZE) {
hid_warn(dev, "pid %d passed too large report\n",
 task_pid_nr(current));
ret = -EINVAL;
goto out;
}


I've just noticed that hid_compute_report_size() doing the same thing as
hid_report_len(). So I will replace it with latter one with length check.

So in [PATCH v2] I will do following:

 1. replace hid_compute_report_size() with hid_report_len()

 2. in hid_report_len() we can hid_warn() if length > HID_MAX_BUFFER_SIZE,
and return HID_MAX_BUFFER_SIZE. Or we can return 0 in hid_report_len() to let
functions like hid_hw_raw_request(), hid_hw_output_report() to validate
invalid report length and return -EINVAL. Though I'll need to add !length
missing checks in other places.

Please let me know what it's preferred way in 2nd step.

[PATCH] drivers/hid: fix for the big hid report length

[Sabyrzhan Tasbolatov]

syzbot found WARNING in hid_alloc_report_buf[1] when the raw buffer for
report is kmalloc() allocated with length > KMALLOC_MAX_SIZE, causing
order >= MAX_ORDER condition:

u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
{
/*
 * 7 extra bytes are necessary to achieve proper functionality
 * of implement() working on 8 byte chunks
 */

u32 len = hid_report_len(report) + 7;

return kmalloc(len, flags);

The restriction with HID_MAX_BUFFER_SIZE (16kb) is, seems, a valid max
limit. I've come up with this in all hid_report_len() xrefs.

The fix inside hid_report_len(), not in *hid_alloc_report_buf() is also
fixing out-of-bounds here in memcpy():

statc int hid_submit_ctrl(..)
{
..
len = hid_report_len(report);
if (dir == USB_DIR_OUT) {
..
if (raw_report) {
memcpy(usbhid->ctrlbuf, raw_report, len);
..

So I've decided to return HID_MAX_BUFFER_SIZE if it the report length is
bigger than limit, otherwise the return the report length.

[1]
Call Trace:
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x40/0x130 mm/slab_common.c:837
 kmalloc_order_trace+0x15/0x70 mm/slab_common.c:853
 kmalloc_large include/linux/slab.h:481 [inline]
 __kmalloc+0x257/0x330 mm/slub.c:3974
 kmalloc include/linux/slab.h:557 [inline]
 hid_alloc_report_buf+0x70/0xa0 drivers/hid/hid-core.c:1648
 __usbhid_submit_report drivers/hid/usbhid/hid-core.c:590 [inline]

Reported-by: syzbot+ab02336a647181a88...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 drivers/hid/usbhid/hid-core.c | 2 +-
 include/linux/hid.h   | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 86257ce6d619..4e9077363c96 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -374,7 +374,7 @@ static int hid_submit_ctrl(struct hid_device *hid)
raw_report = usbhid->ctrl[usbhid->ctrltail].raw_report;
dir = usbhid->ctrl[usbhid->ctrltail].dir;
 
-   len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
+   len = hid_report_len(report);
if (dir == USB_DIR_OUT) {
usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0);
usbhid->urbctrl->transfer_buffer_length = len;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index c39d71eb1fd0..509a6ffdca00 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1156,7 +1156,9 @@ static inline void hid_hw_wait(struct hid_device *hdev)
 static inline u32 hid_report_len(struct hid_report *report)
 {
/* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
-   return ((report->size - 1) >> 3) + 1 + (report->id > 0);
+   u32 len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
+
+   return len > HID_MAX_BUFFER_SIZE ? HID_MAX_BUFFER_SIZE : len;
 }
 
 int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
-- 
2.25.1

[PATCH v2] fs/ext4: fix integer overflow in s_log_groups_per_flex

[Sabyrzhan Tasbolatov]

syzbot found UBSAN: shift-out-of-bounds in ext4_mb_init [1], when
1 << sbi->s_es->s_log_groups_per_flex is bigger than UINT_MAX,
where sbi->s_mb_prefetch is unsigned integer type.

32 is the maximum allowed power of s_log_groups_per_flex. Following if
check will also trigger UBSAN shift-out-of-bound:

if (1 << sbi->s_es->s_log_groups_per_flex >= UINT_MAX) {

So I'm checking it against the raw number, perhaps there is another way
to calculate UINT_MAX max power. Also use min_t as to make sure it's
uint type.

[1] UBSAN: shift-out-of-bounds in fs/ext4/mballoc.c:2713:24
shift exponent 60 is too large for 32-bit type 'int'
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x137/0x1be lib/dump_stack.c:120
 ubsan_epilogue lib/ubsan.c:148 [inline]
 __ubsan_handle_shift_out_of_bounds+0x432/0x4d0 lib/ubsan.c:395
 ext4_mb_init_backend fs/ext4/mballoc.c:2713 [inline]
 ext4_mb_init+0x19bc/0x19f0 fs/ext4/mballoc.c:2898
 ext4_fill_super+0xc2ec/0xfbe0 fs/ext4/super.c:4983

Reported-by: syzbot+a8b4b0c60155e87e9...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
v2: updated > 32 condition to >= 32

> > +   if (sbi->s_es->s_log_groups_per_flex > 32) {
>   ^^ >= 32?
> 
> Otherwise the patch looks good.
> 

Thanks! Updated to >= 32 condition.
---
 fs/ext4/mballoc.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 99bf091fee10..a02fadf4fc84 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2709,8 +2709,15 @@ static int ext4_mb_init_backend(struct super_block *sb)
}
 
if (ext4_has_feature_flex_bg(sb)) {
-   /* a single flex group is supposed to be read by a single IO */
-   sbi->s_mb_prefetch = min(1 << sbi->s_es->s_log_groups_per_flex,
+   /* a single flex group is supposed to be read by a single IO.
+* 2 ^ s_log_groups_per_flex != UINT_MAX as s_mb_prefetch is
+* unsigned integer, so the maximum shift is 32.
+*/
+   if (sbi->s_es->s_log_groups_per_flex >= 32) {
+   ext4_msg(sb, KERN_ERR, "too many log groups per 
flexible block group");
+   goto err_freesgi;
+   }
+   sbi->s_mb_prefetch = min_t(uint, 1 << 
sbi->s_es->s_log_groups_per_flex,
BLK_MAX_SEGMENT_SIZE >> (sb->s_blocksize_bits - 9));
sbi->s_mb_prefetch *= 8; /* 8 prefetch IOs in flight at most */
} else {
-- 
2.25.1

Re: [PATCH] net/qrtr: restrict length in qrtr_tun_write_iter()

[Sabyrzhan Tasbolatov]

> Do we really expect to accept huge lengths here ?

Sorry for late response but I couldnt find any reference to the max
length of incoming data for qrtr TUN interface.

> qrtr_endpoint_post() will later attempt a netdev_alloc_skb() which will need
> some extra space (for struct skb_shared_info)

Thanks, you're right, qrtr_endpoint_post() will alloc another slab buffer.
We can check the length of skb allocation but we need to do following:

int qrtr_endpoint_post(.., const void *data, size_t len) 
{
..
when QRTR_PROTO_VER_1:
hdrlen = sizeof(*data);
when QRTR_PROTO_VER_2:
hdrlen = sizeof(*data) + data->optlen;

len = (KMALLOC_MAX_SIZE - hdrlen) % data->size;
skb = netdev_alloc_skb(NULL, len);
..
skb_put_data(skb, data + hdrlen, size);


So it requires refactoring as in qrtr_tun_write_iter() we just allocate and
pass it to qrtr_endpoint_post() and there
we need to do len calculation as above *before* netdev_alloc_skb(NULL, len).

Perhaps there is a nicer solution though.

Re: WARNING in iov_iter_revert (2)

[Sabyrzhan Tasbolatov]

>--- a/drivers/tty/tty_io.c
>+++ b/drivers/tty/tty_io.c
>@@ -961,6 +961,9 @@ static inline ssize_t do_tty_write(
> ret = write(tty, file, tty->write_buf, size);
> if (ret <= 0)
> break;
>+/* ttyprintk historical oddity */
>+if (ret > size)
>+break;
> 
> /* FIXME! Have Al check this! */
> if (ret != size)
> 
> in there. Because right now we clearly do strange and not-so-wonderful
> things if the write routine returns a bigger value than it was
> passed.. Not limited to that iov_iter_revert() thing, but the whole
> loop.
> 
> Comments?

Just want to comment that this fix is correct (tested),
rather than what I did [1] to return abruptly
in the beginning of do_tty_write() for write(fd, NULL, 0) case.

Let me know if I can prepare a patch with Linus's fix above.

[1] https://lore.kernel.org/lkml/20210217155536.2986178-1-snovit...@gmail.com
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -905,6 +905,9 @@ static inline ssize_t do_tty_write(
ssize_t ret, written = 0;
unsigned int chunk;
 
+   if (!count)
+   return -EFAULT;
+
ret = tty_write_lock(tty, file->f_flags & O_NDELAY);
if (ret < 0)
return ret;

[PATCH v2] tty: fix when iov_iter_count() returns 0 in tty_write()

[Sabyrzhan Tasbolatov]

syzbot found WARNING in iov_iter_revert[1] when iov_iter_count() returns 0,
therefore INT_MAX is passed to iov_iter_revert() causing > MAX_RW_COUNT
warning.

static inline ssize_t do_tty_write()
{
..
size_t count = iov_iter_count(from);
..
size_t size = count;
if (ret != size)
iov_iter_revert(from, size-ret);

[1] WARNING: lib/iov_iter.c:1090
Call Trace:
 do_tty_write drivers/tty/tty_io.c:967 [inline]
 file_tty_write.constprop.0+0x55f/0x8f0 drivers/tty/tty_io.c:1048
 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

Fixes: 9bb48c82aced ("tty: implement write_iter")
Reported-by: syzbot+3d2c27c2b7dc2a948...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---

v2: Fixed "Fixed" tag to proper commit and changed write return to -EFAULT
as this statement is valid, tested via strace:

write(3, NULL, 0)   = -1 EFAULT (Bad address)

Updated to -EFAULT, should be a valid exit code as
copy_from_iter(.., .., from) returns -EFAULT as well if *from is invalid
address.

>
> Nit, you need a ' ' before your '(' character here, otherwise the
> linux-next scripts will complain.

> Also, you got the git commit id wrong, so this needs to be fixed up
> anyway.  You are pointing to a merge point, I doubt that's what you want
> to point to here, right?

Thanks!
---
 drivers/tty/tty_io.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 816e709afa56..e1460cad8b7d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -905,6 +905,9 @@ static inline ssize_t do_tty_write(
ssize_t ret, written = 0;
unsigned int chunk;
 
+   if (!count)
+   return -EFAULT;
+
ret = tty_write_lock(tty, file->f_flags & O_NDELAY);
if (ret < 0)
return ret;
-- 
2.25.1

[PATCH] tty: fix when iov_iter_count() returns 0 in tty_write()

[Sabyrzhan Tasbolatov]

syzbot found WARNING in iov_iter_revert[1] when iov_iter_count() returns 0,
therefore INT_MAX is passed to iov_iter_revert() causing > MAX_RW_COUNT
warning.

static inline ssize_t do_tty_write()
{
..
size_t count = iov_iter_count(from);
..
size_t size = count;
if (ret != size)
iov_iter_revert(from, size-ret);

[1] WARNING: lib/iov_iter.c:1090
Call Trace:
 do_tty_write drivers/tty/tty_io.c:967 [inline]
 file_tty_write.constprop.0+0x55f/0x8f0 drivers/tty/tty_io.c:1048
 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

Fixes: 494e63ee9c("tty: implement write_iter")
Reported-by: syzbot+3d2c27c2b7dc2a948...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 drivers/tty/tty_io.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 816e709afa56..8d6d579ecc3b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -905,6 +905,9 @@ static inline ssize_t do_tty_write(
ssize_t ret, written = 0;
unsigned int chunk;
 
+   if (!count)
+   return -EINVAL;
+
ret = tty_write_lock(tty, file->f_flags & O_NDELAY);
if (ret < 0)
return ret;
-- 
2.25.1

[PATCH v4] drivers/misc/vmw_vmci: restrict too big queue size in qp_host_alloc_queue

[Sabyrzhan Tasbolatov]

syzbot found WARNING in qp_broker_alloc[1] in qp_host_alloc_queue()
when num_pages is 0x11, giving queue_size + queue_page_size
bigger than KMALLOC_MAX_SIZE for kzalloc(), resulting order >= MAX_ORDER
condition.

queue_size + queue_page_size=0x8000d8, where KMALLOC_MAX_SIZE=0x40.

[1]
Call Trace:
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x40/0x130 mm/slab_common.c:837
 kmalloc_order_trace+0x15/0x70 mm/slab_common.c:853
 kmalloc_large include/linux/slab.h:481 [inline]
 __kmalloc+0x257/0x330 mm/slub.c:3959
 kmalloc include/linux/slab.h:557 [inline]
 kzalloc include/linux/slab.h:682 [inline]
 qp_host_alloc_queue drivers/misc/vmw_vmci/vmci_queue_pair.c:540 [inline]
 qp_broker_create drivers/misc/vmw_vmci/vmci_queue_pair.c:1351 [inline]
 qp_broker_alloc+0x936/0x2740 drivers/misc/vmw_vmci/vmci_queue_pair.c:1739

Reported-by: syzbot+15ec7391f3d6a1a7c...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
> This patch does not apply to the tree...
Apologies, it was so stupid from my side.
Tested locally and via syzbot.

v4: made a patch based on commit 65f0d2414b("Merge tag 'sound-5.11-rc4'
of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound")
---
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index c49065887e8f..024dcdbd9d01 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -537,6 +537,9 @@ static struct vmci_queue *qp_host_alloc_queue(u64 size)
 
queue_page_size = num_pages * sizeof(*queue->kernel_if->u.h.page);
 
+   if (queue_size + queue_page_size > KMALLOC_MAX_SIZE)
+   return NULL;
+
queue = kzalloc(queue_size + queue_page_size, GFP_KERNEL);
if (queue) {
queue->q_header = NULL;
-- 
2.25.1

[PATCH v3] drivers/misc/vmw_vmci: restrict too big queue size in

[Sabyrzhan Tasbolatov]

> syzbot found WARNING in qp_broker_alloc[1] in qp_host_alloc_queue()
> when num_pages is 0x11, giving queue_size + queue_page_size
> bigger than KMALLOC_MAX_SIZE for kzalloc(), resulting order >= MAX_ORDER
> condition.
>
> queue_size + queue_page_size=0x8000d8, where KMALLOC_MAX_SIZE=0x40.
>
Reported-by: syzbot+15ec7391f3d6a1a7c...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
> ---
>>> As this is controllable by userspace, you just provided a way to flood
>>> the kernel logs.
>>>
>>> Please make this a dev_dbg() call instead, if you really want to see it.
>>> Otherwise just return NULL, no need to report anything, right?
>> Thanks, removed pr_warn().

>Looks like you forgot to take out the opening brace.

Cringe moment. Sorry, should've checked it properly first.

v3: Removed opening brace.
---
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index ea16df73cde0..024dcdbd9d01 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -537,7 +537,7 @@ static struct vmci_queue *qp_host_alloc_queue(u64 size)
 
queue_page_size = num_pages * sizeof(*queue->kernel_if->u.h.page);
 
-   if (queue_size + queue_page_size > KMALLOC_MAX_SIZE) {
+   if (queue_size + queue_page_size > KMALLOC_MAX_SIZE)
return NULL;
 
queue = kzalloc(queue_size + queue_page_size, GFP_KERNEL);
-- 
2.25.1

[PATCH v2] drivers/misc/vmw_vmci: restrict too big queue size in

[Sabyrzhan Tasbolatov]

syzbot found WARNING in qp_broker_alloc[1] in qp_host_alloc_queue()
when num_pages is 0x11, giving queue_size + queue_page_size
bigger than KMALLOC_MAX_SIZE for kzalloc(), resulting order >= MAX_ORDER
condition.

queue_size + queue_page_size=0x8000d8, where KMALLOC_MAX_SIZE=0x40.

Reported-by: syzbot+15ec7391f3d6a1a7c...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
>As this is controllable by userspace, you just provided a way to flood
>the kernel logs.
>
>Please make this a dev_dbg() call instead, if you really want to see it.
>Otherwise just return NULL, no need to report anything, right?

Thanks, removed pr_warn().

v2: Removed pr_warn() to avoid flood from user-space
---
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index f6af406fda80..ea16df73cde0 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -538,9 +538,7 @@ static struct vmci_queue *qp_host_alloc_queue(u64 size)
queue_page_size = num_pages * sizeof(*queue->kernel_if->u.h.page);
 
if (queue_size + queue_page_size > KMALLOC_MAX_SIZE) {
-   pr_warn("too big queue to allocate\n");
return NULL;
-   }
 
queue = kzalloc(queue_size + queue_page_size, GFP_KERNEL);
if (queue) {
-- 
2.25.1

[PATCH] drivers/misc/vmw_vmci: restrict too big queue size in qp_host_alloc_queue

[Sabyrzhan Tasbolatov]

syzbot found WARNING in qp_broker_alloc[1] in qp_host_alloc_queue()
when num_pages is 0x11, giving queue_size + queue_page_size
bigger than KMALLOC_MAX_SIZE for kzalloc(), resulting order >= MAX_ORDER
condition.

queue_size + queue_page_size=0x8000d8, where KMALLOC_MAX_SIZE=0x40.


FYI, I've also noticed in vmci_queue_pair.c other SLAB allocations with no
length check that might exceed KMALLOC_MAX_SIZE as well,
but syzbot doesn't have reproduces for them.

in qp_alloc_ppn_set():
produce_ppns =
kmalloc_array(num_produce_pages, sizeof(*produce_ppns),
  GFP_KERNEL);
[..]
consume_ppns =
kmalloc_array(num_consume_pages, sizeof(*consume_ppns),
  GFP_KERNEL);
[..]
in qp_alloc_hypercall():
msg_size = sizeof(*alloc_msg) +
(size_t) entry->num_ppns * ppn_size;
alloc_msg = kmalloc(msg_size, GFP_KERNEL);
[..]
in qp_broker_create():
entry->local_mem = kcalloc(QPE_NUM_PAGES(entry->qp),
   PAGE_SIZE, GFP_KERNEL);

[1]
Call Trace:
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x40/0x130 mm/slab_common.c:837
 kmalloc_order_trace+0x15/0x70 mm/slab_common.c:853
 kmalloc_large include/linux/slab.h:481 [inline]
 __kmalloc+0x257/0x330 mm/slub.c:3959
 kmalloc include/linux/slab.h:557 [inline]
 kzalloc include/linux/slab.h:682 [inline]
 qp_host_alloc_queue drivers/misc/vmw_vmci/vmci_queue_pair.c:540 [inline]
 qp_broker_create drivers/misc/vmw_vmci/vmci_queue_pair.c:1351 [inline]
 qp_broker_alloc+0x936/0x2740 drivers/misc/vmw_vmci/vmci_queue_pair.c:1739

Reported-by: syzbot+15ec7391f3d6a1a7c...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 drivers/misc/vmw_vmci/vmci_queue_pair.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c 
b/drivers/misc/vmw_vmci/vmci_queue_pair.c
index c49065887e8f..f6af406fda80 100644
--- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
+++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
@@ -537,6 +537,11 @@ static struct vmci_queue *qp_host_alloc_queue(u64 size)
 
queue_page_size = num_pages * sizeof(*queue->kernel_if->u.h.page);
 
+   if (queue_size + queue_page_size > KMALLOC_MAX_SIZE) {
+   pr_warn("too big queue to allocate\n");
+   return NULL;
+   }
+
queue = kzalloc(queue_size + queue_page_size, GFP_KERNEL);
if (queue) {
queue->q_header = NULL;
-- 
2.25.1

[PATCH] net/qrtr: replaced useless kzalloc with kmalloc in qrtr_tun_write_iter()

[Sabyrzhan Tasbolatov]

Replaced kzalloc() with kmalloc(), there is no need for zeroed-out
memory for simple void *kbuf.

>For potential, separate clean up - this is followed 
>by copy_from_iter_full(len) kzalloc() can probably 
>be replaced by kmalloc()?
>
>>  if (!kbuf)
>>  return -ENOMEM;

Signed-off-by: Sabyrzhan Tasbolatov 
---
 net/qrtr/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
index b238c40a9984..9b607c7614de 100644
--- a/net/qrtr/tun.c
+++ b/net/qrtr/tun.c
@@ -86,7 +86,7 @@ static ssize_t qrtr_tun_write_iter(struct kiocb *iocb, struct 
iov_iter *from)
if (len > KMALLOC_MAX_SIZE)
return -ENOMEM;
 
-   kbuf = kzalloc(len, GFP_KERNEL);
+   kbuf = kmalloc(len, GFP_KERNEL);
if (!kbuf)
return -ENOMEM;
 
-- 
2.25.1

[PATCH v2] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[Sabyrzhan Tasbolatov]

In PATCH v2 fixed return -ENOMEM as error pointer.

>syzbot found WARNING in squashfs_read_table [1] when length of xattr_ids
>exceeds KMALLOC_MAX_SIZE in squashfs_read_table() for kmalloc().
>
>For other squashfs tables, currently such as boundary is checked with
>another table's boundaries. Xattr table is the last one, so there is
>no defined limit. But to avoid order >= MAX_ORDER warning condition,
>we should restrict SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids) to
>KMALLOC_MAX_SIZE, and it gives 1024 pages in squashfs_read_table via
>(length + PAGE_SIZE - 1) >> PAGE_SHIFT.
>
>[1]
>Call Trace:
> alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
> alloc_pages include/linux/gfp.h:547 [inline]
> kmalloc_order+0x2e/0xb0 mm/slab_common.c:916
> kmalloc_order_trace+0x14/0x120 mm/slab_common.c:932
> kmalloc include/linux/slab.h:559 [inline]
> squashfs_read_table+0x43/0x1e0 fs/squashfs/cache.c:413
> squashfs_read_xattr_id_table+0x191/0x220 fs/squashfs/xattr_id.c:81

Reported-by: syzbot+2ccea6339d3683608...@syzkaller.appspotmail.com
Reported-by: kernel test robot 
Signed-off-by: Sabyrzhan Tasbolatov 
---
 fs/squashfs/xattr_id.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/squashfs/xattr_id.c b/fs/squashfs/xattr_id.c
index d99e08464554..2462876c66c4 100644
--- a/fs/squashfs/xattr_id.c
+++ b/fs/squashfs/xattr_id.c
@@ -78,5 +78,8 @@ __le64 *squashfs_read_xattr_id_table(struct super_block *sb, 
u64 start,
 
TRACE("In read_xattr_index_table, length %d\n", len);
 
+   if (len > KMALLOC_MAX_SIZE)
+   return ERR_PTR(-ENOMEM);
+
return squashfs_read_table(sb, start + sizeof(*id_table), len);
 }
-- 
2.25.1

[PATCH] fs/ext4: fix integer overflow in s_log_groups_per_flex

[Sabyrzhan Tasbolatov]

syzbot found UBSAN: shift-out-of-bounds in ext4_mb_init [1], when
1 << sbi->s_es->s_log_groups_per_flex is bigger than UINT_MAX,
where sbi->s_mb_prefetch is unsigned integer type.

32 is the maximum allowed power of s_log_groups_per_flex. Following if
check will also trigger UBSAN shift-out-of-bound:

if (1 << sbi->s_es->s_log_groups_per_flex > UINT_MAX) {

So I'm checking it against the raw number, perhaps there is another way
to calculate UINT_MAX max power. Also use min_t as to make sure it's
uint type.

[1] UBSAN: shift-out-of-bounds in fs/ext4/mballoc.c:2713:24
shift exponent 60 is too large for 32-bit type 'int'
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x137/0x1be lib/dump_stack.c:120
 ubsan_epilogue lib/ubsan.c:148 [inline]
 __ubsan_handle_shift_out_of_bounds+0x432/0x4d0 lib/ubsan.c:395
 ext4_mb_init_backend fs/ext4/mballoc.c:2713 [inline]
 ext4_mb_init+0x19bc/0x19f0 fs/ext4/mballoc.c:2898
 ext4_fill_super+0xc2ec/0xfbe0 fs/ext4/super.c:4983

Reported-by: syzbot+a8b4b0c60155e87e9...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 fs/ext4/mballoc.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 99bf091fee10..e1e7ffbba1a6 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2709,8 +2709,15 @@ static int ext4_mb_init_backend(struct super_block *sb)
}
 
if (ext4_has_feature_flex_bg(sb)) {
-   /* a single flex group is supposed to be read by a single IO */
-   sbi->s_mb_prefetch = min(1 << sbi->s_es->s_log_groups_per_flex,
+   /* a single flex group is supposed to be read by a single IO.
+* 2 ^ s_log_groups_per_flex != UINT_MAX as s_mb_prefetch is
+* unsigned integer, so the maximum shift is 32.
+*/
+   if (sbi->s_es->s_log_groups_per_flex > 32) {
+   ext4_msg(sb, KERN_ERR, "too many log groups per 
flexible block group");
+   goto err_freesgi;
+   }
+   sbi->s_mb_prefetch = min_t(uint, 1 << 
sbi->s_es->s_log_groups_per_flex,
BLK_MAX_SEGMENT_SIZE >> (sb->s_blocksize_bits - 9));
sbi->s_mb_prefetch *= 8; /* 8 prefetch IOs in flight at most */
} else {
-- 
2.25.1

[PATCH] fs/squashfs: restrict length of xattr_ids in read_xattr_id_table

[Sabyrzhan Tasbolatov]

syzbot found WARNING in squashfs_read_table [1] when length of xattr_ids
exceeds KMALLOC_MAX_SIZE in squashfs_read_table() for kmalloc().

For other squashfs tables, currently such as boundary is checked with
another table's boundaries. Xattr table is the last one, so there is
no defined limit. But to avoid order >= MAX_ORDER warning condition,
we should restrict SQUASHFS_XATTR_BLOCK_BYTES(*xattr_ids) to
KMALLOC_MAX_SIZE, and it gives 1024 pages in squashfs_read_table via
(length + PAGE_SIZE - 1) >> PAGE_SHIFT.

[1]
Call Trace:
 alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x2e/0xb0 mm/slab_common.c:916
 kmalloc_order_trace+0x14/0x120 mm/slab_common.c:932
 kmalloc include/linux/slab.h:559 [inline]
 squashfs_read_table+0x43/0x1e0 fs/squashfs/cache.c:413
 squashfs_read_xattr_id_table+0x191/0x220 fs/squashfs/xattr_id.c:81

Reported-by: syzbot+2ccea6339d3683608...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 fs/squashfs/xattr_id.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/squashfs/xattr_id.c b/fs/squashfs/xattr_id.c
index d99e08464554..6bb51cd3d5c1 100644
--- a/fs/squashfs/xattr_id.c
+++ b/fs/squashfs/xattr_id.c
@@ -78,5 +78,8 @@ __le64 *squashfs_read_xattr_id_table(struct super_block *sb, 
u64 start,
 
TRACE("In read_xattr_index_table, length %d\n", len);
 
+   if (len > KMALLOC_MAX_SIZE)
+   return -ENOMEM;
+
return squashfs_read_table(sb, start + sizeof(*id_table), len);
 }
-- 
2.25.1

Re: [PATCH v2] smackfs: restrict bytes count in smackfs write functions

[Sabyrzhan Tasbolatov]

> if PAGE_SIZE >= SMK_LOADSIZE all legitimate requests can be made
> using PAGE_SIZE as a limit. Your example with 19990 spaces before
> the data demonstrates that the interface is inadequately documented.
> Tizen and Automotive Grade Linux are going to be fine with a PAGE_SIZE
> limit. The best way to address this concern is to go ahead with the
> PAGE_SIZE limit and create ABI documents for the smackfs interfaces.
> I will take your patch for the former and create a patch for the latter.

Please let me know if there is anything else required for this patch.
AFAIU, we agreed with PAGE_SIZE as the limit.

[PATCH] drivers/mt: restrict length of kzalloc in input_mt_init_slots()

[Sabyrzhan Tasbolatov]

syzbot found WARNING in input_mt_init_slots [1] when
struct_size(mt, slots, num_slots)=0x40006 where num_slots=0x10001,
which exceeds KMALLOC_MAX_SIZE (0x4) and causes
order >= MAX_ORDER condition.

[1]
Call Trace:
 alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x2e/0xb0 mm/slab_common.c:837
 kmalloc_order_trace+0x14/0x120 mm/slab_common.c:853
 kmalloc include/linux/slab.h:557 [inline]
 kzalloc include/linux/slab.h:682 [inline]
 input_mt_init_slots drivers/input/input-mt.c:49 [inline]

Reported-by: syzbot+0122fa359a6969439...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 drivers/input/input-mt.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index 44fe6f2f063c..e542f45a45ab 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -40,13 +40,18 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int 
num_slots,
 {
struct input_mt *mt = dev->mt;
int i;
+   size_t mt_size = 0;
 
if (!num_slots)
return 0;
if (mt)
return mt->num_slots != num_slots ? -EINVAL : 0;
 
-   mt = kzalloc(struct_size(mt, slots, num_slots), GFP_KERNEL);
+   mt_size = struct_size(mt, slots, num_slots);
+   if (mt_size > KMALLOC_MAX_SIZE)
+   return -ENOMEM;
+
+   mt = kzalloc(mt_size, GFP_KERNEL);
if (!mt)
goto err_mem;
 
-- 
2.25.1

[PATCH] net/qrtr: restrict user-controlled length in qrtr_tun_write_iter()

[Sabyrzhan Tasbolatov]

syzbot found WARNING in qrtr_tun_write_iter [1] when write_iter length
exceeds KMALLOC_MAX_SIZE causing order >= MAX_ORDER condition.

Additionally, there is no check for 0 length write.

[1]
WARNING: mm/page_alloc.c:5011
[..]
Call Trace:
 alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x2e/0xb0 mm/slab_common.c:837
 kmalloc_order_trace+0x14/0x120 mm/slab_common.c:853
 kmalloc include/linux/slab.h:557 [inline]
 kzalloc include/linux/slab.h:682 [inline]
 qrtr_tun_write_iter+0x8a/0x180 net/qrtr/tun.c:83
 call_write_iter include/linux/fs.h:1901 [inline]

Reported-by: syzbot+c2a7e5c5211605a90...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 net/qrtr/tun.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/qrtr/tun.c b/net/qrtr/tun.c
index 15ce9b642b25..b238c40a9984 100644
--- a/net/qrtr/tun.c
+++ b/net/qrtr/tun.c
@@ -80,6 +80,12 @@ static ssize_t qrtr_tun_write_iter(struct kiocb *iocb, 
struct iov_iter *from)
ssize_t ret;
void *kbuf;
 
+   if (!len)
+   return -EINVAL;
+
+   if (len > KMALLOC_MAX_SIZE)
+   return -ENOMEM;
+
kbuf = kzalloc(len, GFP_KERNEL);
if (!kbuf)
return -ENOMEM;
-- 
2.25.1

[PATCH] net/rds: restrict iovecs length for RDS_CMSG_RDMA_ARGS

[Sabyrzhan Tasbolatov]

syzbot found WARNING in rds_rdma_extra_size [1] when RDS_CMSG_RDMA_ARGS
control message is passed with user-controlled
0x40001 bytes of args->nr_local, causing order >= MAX_ORDER condition.

The exact value 0x40001 can be checked with UIO_MAXIOV which is 0x400.
So for kcalloc() 0x400 iovecs with sizeof(struct rds_iovec) = 0x10
is the closest limit, with 0x10 leftover.

Same condition is currently done in rds_cmsg_rdma_args().

[1] WARNING: mm/page_alloc.c:5011
[..]
Call Trace:
 alloc_pages_current+0x18c/0x2a0 mm/mempolicy.c:2267
 alloc_pages include/linux/gfp.h:547 [inline]
 kmalloc_order+0x2e/0xb0 mm/slab_common.c:837
 kmalloc_order_trace+0x14/0x120 mm/slab_common.c:853
 kmalloc_array include/linux/slab.h:592 [inline]
 kcalloc include/linux/slab.h:621 [inline]
 rds_rdma_extra_size+0xb2/0x3b0 net/rds/rdma.c:568
 rds_rm_size net/rds/send.c:928 [inline]

Reported-by: syzbot+1bd2b07f93745fa38...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 net/rds/rdma.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/rds/rdma.c b/net/rds/rdma.c
index 1d0afb1dd77b..6f1a50d50d06 100644
--- a/net/rds/rdma.c
+++ b/net/rds/rdma.c
@@ -565,6 +565,9 @@ int rds_rdma_extra_size(struct rds_rdma_args *args,
if (args->nr_local == 0)
return -EINVAL;
 
+   if (args->nr_local > UIO_MAXIOV)
+   return -EMSGSIZE;
+
iov->iov = kcalloc(args->nr_local,
   sizeof(struct rds_iovec),
   GFP_KERNEL);
-- 
2.25.1

Re: [PATCH v2] smackfs: restrict bytes count in smackfs write functions

[Sabyrzhan Tasbolatov]

> > /*
> > +* No partial write.
> >  * Enough data must be present.
> >  */
> > if (*ppos != 0)
> > return -EINVAL;
> > +   if (count == 0 || count > PAGE_SIZE)
> > +   return -EINVAL;
> >  
> > data = memdup_user_nul(buf, count);
> > if (IS_ERR(data))
> > 
> 
> Doesn't this change break legitimate requests like
> 
>   char buffer[2];
> 
>   memset(buffer, ' ', sizeof(buffer));
>   memcpy(buffer + sizeof(buffer) - 10, "foo", 3);
>   write(fd, buffer, sizeof(buffer));
> 
> ?

It does, in this case. Then I need to patch another version with
whitespace stripping before, after label. I just followed the same thing
that I see in security/selinux/selinuxfs.c sel_write_enforce() etc.

It has the same memdup_user_nul() and count >= PAGE_SIZE check prior to that.

[PATCH v2] smackfs: restrict bytes count in smackfs write functions

[Sabyrzhan Tasbolatov]

syzbot found WARNINGs in several smackfs write operations where
bytes count is passed to memdup_user_nul which exceeds
GFP MAX_ORDER. Check count size if bigger than PAGE_SIZE.

Per smackfs doc, smk_write_net4addr accepts any label or -CIPSO,
smk_write_net6addr accepts any label or -DELETE. I couldn't find
any general rule for other label lengths except SMK_LABELLEN,
SMK_LONGLABEL, SMK_CIPSOMAX which are documented.

Let's constrain, in general, smackfs label lengths for PAGE_SIZE.
Although fuzzer crashes write to smackfs/netlabel on 0x40 length.

Here is a quick way to reproduce the WARNING:
python -c "print('A' * 0x40)" > /sys/fs/smackfs/netlabel

Reported-by: syzbot+a71a442385a0b2815...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 security/smack/smackfs.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 5d44b7d258ef..22ded2c26089 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1167,7 +1167,7 @@ static ssize_t smk_write_net4addr(struct file *file, 
const char __user *buf,
return -EPERM;
if (*ppos != 0)
return -EINVAL;
-   if (count < SMK_NETLBLADDRMIN)
+   if (count < SMK_NETLBLADDRMIN || count > PAGE_SIZE - 1)
return -EINVAL;
 
data = memdup_user_nul(buf, count);
@@ -1427,7 +1427,7 @@ static ssize_t smk_write_net6addr(struct file *file, 
const char __user *buf,
return -EPERM;
if (*ppos != 0)
return -EINVAL;
-   if (count < SMK_NETLBLADDRMIN)
+   if (count < SMK_NETLBLADDRMIN || count > PAGE_SIZE - 1)
return -EINVAL;
 
data = memdup_user_nul(buf, count);
@@ -1834,6 +1834,10 @@ static ssize_t smk_write_ambient(struct file *file, 
const char __user *buf,
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
 
+   /* Enough data must be present */
+   if (count == 0 || count > PAGE_SIZE)
+   return -EINVAL;
+
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
return PTR_ERR(data);
@@ -2005,6 +2009,9 @@ static ssize_t smk_write_onlycap(struct file *file, const 
char __user *buf,
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
 
+   if (count > PAGE_SIZE)
+   return -EINVAL;
+
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
return PTR_ERR(data);
@@ -2092,6 +2099,9 @@ static ssize_t smk_write_unconfined(struct file *file, 
const char __user *buf,
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
 
+   if (count > PAGE_SIZE)
+   return -EINVAL;
+
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
return PTR_ERR(data);
@@ -2648,6 +2658,10 @@ static ssize_t smk_write_syslog(struct file *file, const 
char __user *buf,
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
 
+   /* Enough data must be present */
+   if (count == 0 || count > PAGE_SIZE)
+   return -EINVAL;
+
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
return PTR_ERR(data);
@@ -2740,10 +2754,13 @@ static ssize_t smk_write_relabel_self(struct file 
*file, const char __user *buf,
return -EPERM;
 
/*
+* No partial write.
 * Enough data must be present.
 */
if (*ppos != 0)
return -EINVAL;
+   if (count == 0 || count > PAGE_SIZE)
+   return -EINVAL;
 
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
-- 
2.25.1

[PATCH] smackfs: restrict bytes count in smackfs write functions

[Sabyrzhan Tasbolatov]

syzbot found WARNINGs in several smackfs write operations where
bytes count is passed to memdup_user_nul which exceeds
GFP MAX_ORDER. Check count size if bigger SMK_LONGLABEL,
for smk_write_syslog if bigger than PAGE_SIZE - 1.

Reported-by: syzbot+a71a442385a0b2815...@syzkaller.appspotmail.com
Signed-off-by: Sabyrzhan Tasbolatov 
---
 security/smack/smackfs.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 5d44b7d258ef..88678c6f1b8c 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1167,7 +1167,7 @@ static ssize_t smk_write_net4addr(struct file *file, 
const char __user *buf,
return -EPERM;
if (*ppos != 0)
return -EINVAL;
-   if (count < SMK_NETLBLADDRMIN)
+   if (count < SMK_NETLBLADDRMIN || count > SMK_LONGLABEL)
return -EINVAL;
 
data = memdup_user_nul(buf, count);
@@ -1427,7 +1427,7 @@ static ssize_t smk_write_net6addr(struct file *file, 
const char __user *buf,
return -EPERM;
if (*ppos != 0)
return -EINVAL;
-   if (count < SMK_NETLBLADDRMIN)
+   if (count < SMK_NETLBLADDRMIN || count > SMK_LONGLABEL)
return -EINVAL;
 
data = memdup_user_nul(buf, count);
@@ -2647,6 +2647,8 @@ static ssize_t smk_write_syslog(struct file *file, const 
char __user *buf,
 
if (!smack_privileged(CAP_MAC_ADMIN))
return -EPERM;
+   if (count > PAGE_SIZE - 1)
+   return -EINVAL;
 
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
@@ -2744,6 +2746,8 @@ static ssize_t smk_write_relabel_self(struct file *file, 
const char __user *buf,
 */
if (*ppos != 0)
return -EINVAL;
+   if (count > SMK_LONGLABEL)
+   return -EINVAL;
 
data = memdup_user_nul(buf, count);
if (IS_ERR(data))
-- 
2.25.1