[PATCH v3 4/4] mtdchar: add MEMREAD ioctl
Michał Kępień
kernel at kempniu.pl
Mon Feb 14 03:15:02 PST 2022
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ń
More information about the linux-mtd
mailing list