lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 14 Jan 2019 17:13:21 -0800 From: Kees Cook <keescook@...omium.org> To: Willy Tarreau <w@....eu> Cc: Silvio Cesare <silvio.cesare@...il.com>, LKML <linux-kernel@...r.kernel.org>, Timur Tabi <timur@...nel.org>, Nicolin Chen <nicoleotsuka@...il.com>, Xiubo Li <Xiubo.Lee@...il.com>, Fabio Estevam <fabio.estevam@....com>, Dan Carpenter <dan.carpenter@...cle.com>, Will Deacon <will.deacon@....com>, Greg KH <greg@...ah.com> Subject: Re: [PATCH 4/8] ASoC: change snprintf to scnprintf for possible overflow On Sat, Jan 12, 2019 at 7:28 AM Willy Tarreau <w@....eu> wrote: > > From: Silvio Cesare <silvio.cesare@...il.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@...il.com> > Cc: Timur Tabi <timur@...nel.org> > Cc: Nicolin Chen <nicoleotsuka@...il.com> > Cc: Xiubo Li <Xiubo.Lee@...il.com> > Cc: Fabio Estevam <fabio.estevam@....com> > Cc: Dan Carpenter <dan.carpenter@...cle.com> > Cc: Kees Cook <keescook@...omium.org> > Cc: Will Deacon <will.deacon@....com> > Cc: Greg KH <greg@...ah.com> > Signed-off-by: Willy Tarreau <w@....eu> Reviewed-by: Kees Cook <keescook@...omium.org> -Kees > > --- > sound/soc/fsl/imx-audmux.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c > index 392d5eef356d..99e07b01a2ce 100644 > --- a/sound/soc/fsl/imx-audmux.c > +++ b/sound/soc/fsl/imx-audmux.c > @@ -86,49 +86,49 @@ static ssize_t audmux_read_file(struct file *file, char __user *user_buf, > if (!buf) > return -ENOMEM; > > - ret = snprintf(buf, PAGE_SIZE, "PDCR: %08x\nPTCR: %08x\n", > + ret = scnprintf(buf, PAGE_SIZE, "PDCR: %08x\nPTCR: %08x\n", > pdcr, ptcr); > > if (ptcr & IMX_AUDMUX_V2_PTCR_TFSDIR) > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "TxFS output from %s, ", > audmux_port_string((ptcr >> 27) & 0x7)); > else > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "TxFS input, "); > > if (ptcr & IMX_AUDMUX_V2_PTCR_TCLKDIR) > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "TxClk output from %s", > audmux_port_string((ptcr >> 22) & 0x7)); > else > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "TxClk input"); > > - ret += snprintf(buf + ret, PAGE_SIZE - ret, "\n"); > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n"); > > if (ptcr & IMX_AUDMUX_V2_PTCR_SYN) { > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "Port is symmetric"); > } else { > if (ptcr & IMX_AUDMUX_V2_PTCR_RFSDIR) > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "RxFS output from %s, ", > audmux_port_string((ptcr >> 17) & 0x7)); > else > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "RxFS input, "); > > if (ptcr & IMX_AUDMUX_V2_PTCR_RCLKDIR) > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "RxClk output from %s", > audmux_port_string((ptcr >> 12) & 0x7)); > else > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "RxClk input"); > } > > - ret += snprintf(buf + ret, PAGE_SIZE - ret, > + ret += scnprintf(buf + ret, PAGE_SIZE - ret, > "\nData received from %s\n", > audmux_port_string((pdcr >> 13) & 0x7)); > > -- > 2.19.2 > -- Kees Cook
Powered by blists - more mailing lists