Messages in this thread | | | From | Kalle Valo <> | Subject | Re: [PATCH 2/8] libertas: change snprintf to scnprintf for possible overflow | Date | Wed, 16 Jan 2019 18:40:29 +0200 |
| |
Willy Tarreau <w@1wt.eu> writes:
> On Tue, Jan 15, 2019 at 07:55:36AM +0200, Kalle Valo wrote: >> Willy Tarreau <w@1wt.eu> writes: >> >> > From: Silvio Cesare <silvio.cesare@gmail.com> >> > >> > Change snprintf to scnprintf. There are generally two cases where using >> > snprintf causes problems. >> > >> > 1) Uses of size += snprintf(buf, SIZE - size, fmt, ...) >> > In this case, if snprintf would have written more characters than what the >> > buffer size (SIZE) is, then size will end up larger than SIZE. In later >> > uses of snprintf, SIZE - size will result in a negative number, leading >> > to problems. Note that size might already be too large by using >> > size = snprintf before the code reaches a case of size += snprintf. >> > >> > 2) If size is ultimately used as a length parameter for a copy back to user >> > space, then it will potentially allow for a buffer overflow and information >> > disclosure when size is greater than SIZE. When the size is used to index >> > the buffer directly, we can have memory corruption. This also means when >> > size = snprintf... is used, it may also cause problems since size may become >> > large. Copying to userspace is mitigated by the HARDENED_USERCOPY kernel >> > configuration. >> > >> > The solution to these issues is to use scnprintf which returns the number of >> > characters actually written to the buffer, so the size variable will never >> > exceed SIZE. >> > >> > Signed-off-by: Silvio Cesare <silvio.cesare@gmail.com> >> > Cc: Kalle Valo <kvalo@codeaurora.org> >> > Cc: Dan Carpenter <dan.carpenter@oracle.com> >> > Cc: Kees Cook <keescook@chromium.org> >> > Cc: Will Deacon <will.deacon@arm.com> >> > Cc: Greg KH <greg@kroah.com> >> > Signed-off-by: Willy Tarreau <w@1wt.eu> >> >> I don't see any mention about which tree this should go to. Can I take >> this to wireless-drivers-next? > > Possibly. It addresses a small memory disclosure issue when using debugfs, > and as such it should probably also be submitted to stable branches, so > please use the most suitable tree that doesn't add too much extra delay.
Ok, I'll queue this for 5.0 and apply it to wireless-drivers instead.
-- Kalle Valo
| |