2022-07-06 07:40:32

by Dan Carpenter

Subject: [PATCH] eeprom: idt_89hpesx: uninitialized data in idt_dbgfs_csr_write()

The simple_write_to_buffer() here is used to copy a string from the
user. However the function call will succeed if it manages to read
just one byte anywhere within the buffer while we need the whole string
to be initialized to avoid using uninitialized data.

Fixes: cfad6425382e ("eeprom: Add IDT 89HPESx EEPROM/CSR driver")
Signed-off-by: Dan Carpenter <[email protected]>
---
drivers/misc/eeprom/idt_89hpesx.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
index b0cff4b152da..30e146b32205 100644
--- a/drivers/misc/eeprom/idt_89hpesx.c
+++ b/drivers/misc/eeprom/idt_89hpesx.c
@@ -909,15 +909,13 @@ static ssize_t idt_dbgfs_csr_write(struct file *filep, const char __user *ubuf,
u32 csraddr, csrval;
char *buf;

- /* Copy data from User-space */
- buf = kmalloc(count + 1, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ if (*offp != 0)
+ return -EINVAL;

- ret = simple_write_to_buffer(buf, count, offp, ubuf, count);
- if (ret < 0)
- goto free_buf;
- buf[count] = 0;
+ /* Copy data from User-space */
+ buf = strndup_user(ubuf, count + 1);
+ if (IS_ERR(buf))
+ return PTR_ERR(buf);

/* Find position of colon in the buffer */
colon_ch = strnchr(buf, count, ':');
--
2.35.1

2022-07-06 11:07:10

by Dan Carpenter

Subject: Re: [PATCH] eeprom: idt_89hpesx: uninitialized data in idt_dbgfs_csr_write()

On Wed, Jul 06, 2022 at 09:58:51AM +0300, Dan Carpenter wrote:
> The simple_write_to_buffer() here is used to copy a string from the
> user. However the function call will succeed if it manages to read
> just one byte anywhere within the buffer while we need the whole string
> to be initialized to avoid using uninitialized data.
>
> Fixes: cfad6425382e ("eeprom: Add IDT 89HPESx EEPROM/CSR driver")
> Signed-off-by: Dan Carpenter <[email protected]>
> ---

NAK. The strndup_user() function is too strict. I will resend this
using memdup_user() instead.

regards,
dan carpenter

2022-07-07 18:34:41

by Serge Semin

Subject: Re: [PATCH] eeprom: idt_89hpesx: uninitialized data in idt_dbgfs_csr_write()

Hi Dan

On Wed, Jul 06, 2022 at 01:40:24PM +0300, Dan Carpenter wrote:
> On Wed, Jul 06, 2022 at 09:58:51AM +0300, Dan Carpenter wrote:
> > The simple_write_to_buffer() here is used to copy a string from the
> > user. However the function call will succeed if it manages to read
> > just one byte anywhere within the buffer while we need the whole string
> > to be initialized to avoid using uninitialized data.
> >
> > Fixes: cfad6425382e ("eeprom: Add IDT 89HPESx EEPROM/CSR driver")
> > Signed-off-by: Dan Carpenter <[email protected]>
> > ---
>
> NAK. The strndup_user() function is too strict. I will resend this
> using memdup_user() instead.

Thanks for the patch. Ok. I'll wait for v2 to review.

-Sergey

>
> regards,
> dan carpenter
>