lkml.org 
[lkml]   [2022]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 4/4] mtdchar: add MEMREAD ioctl
Richard,

Thank you for taking a look at this patch series.

> > + if (req.start + req.len > mtd->size) {
>
> I think this can overflow since both req.start and req.len are u64.
> So an evil-doer might bypass this check.

You are right, thanks. I adopted this check from mtd_check_oob_ops()
and your comment made me think that maybe the MEMREADOOB64/MEMWRITEOOB64
ioctls are affected as well, but it looks like 'len' is a 32-bit integer
in those other cases, so they look safe to me.

However, the MEMWRITE ioctl does seem to be affected by the same issue
since commit f6562bca84d22525f792305e3106571f8714d057 ("mtdchar: prevent
unbounded allocation in MEMWRITE ioctl"), see mtdchar_write_ioctl().

Changing the 'len' and 'ooblen' fields of struct mtd_{read,write}_req to
u32 would break userspace, so that is not an option. Would truncating
req.len to 32 bits (req.len &= 0xffffffff) early in the two relevant
functions be the way to go? I guess such a change should be reflected
in include/uapi/mtd/mtd-abi.h, too.

--
Best regards,
Michał Kępień

\
 
 \ /
  Last update: 2022-02-14 12:33    [W:0.072 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site