On Tue, Oct 15, 2024 at 4:55 PM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote: > > On 10/15/24 16:48, Everest K.C. wrote: > > On Tue, Oct 15, 2024 at 4:35 PM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote: > >> > >> On 10/15/24 15:51, Everest K.C. wrote: > >>> The variable `in_data` is freed, but used later in the code. > >>> Fix it by moving the freeing the memory after it use has been > >>> completed. > >>> > >>> This issue was reported by Coverity Scan. > >>> Report: > >>> CID 1600783: (#1 of 1): Use after free (USE_AFTER_FREE) > >>> 19. pass_freed_arg: Passing freed pointer in_data as an argument to > >>> ni_usb_dump_raw_block. > >>> > >>> Fixes: 4e127de14fa7 ("staging: gpib: Add National Instruments USB GPIB driver") > >>> Signed-off-by: Everest K.C. <everestkc@xxxxxxxxxxxxxxxx> > >>> --- > >>> drivers/staging/gpib/ni_usb/ni_usb_gpib.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/staging/gpib/ni_usb/ni_usb_gpib.c b/drivers/staging/gpib/ni_usb/ni_usb_gpib.c > >>> index 1da263676f2a..75f39e1f3ed1 100644 > >>> --- a/drivers/staging/gpib/ni_usb/ni_usb_gpib.c > >>> +++ b/drivers/staging/gpib/ni_usb/ni_usb_gpib.c > >>> @@ -690,12 +690,12 @@ static int ni_usb_read(gpib_board_t *board, uint8_t *buffer, size_t length, > >>> kfree(in_data); > >>> return parse_retval; > >>> } > >>> - kfree(in_data); > >>> if (actual_length != length - status.count) { > >>> pr_err("%s: actual_length=%i expected=%li\n", > >>> __func__, actual_length, (long)(length - status.count)); > >>> ni_usb_dump_raw_block(in_data, usb_bytes_read); > >>> } > >>> + kfree(in_data); > >>> switch (status.error_code) { > >>> case NIUSB_NO_ERROR: > >>> retval = 0; > >> > >> Looks good to me. Isn't this on next though. Don't forget to > >> indicate it is against next. > > No, it was fixed in the linux-staging repo. > >> Reviewed-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> > >> > > Okay - by the way the same problem is in ni_usb_write_registers(). > Did coverity catch that one? No, there were no similar issues caught by coverity. Also, I just checked the function. I didn't see the variable being accessed after it had been freed. > Both problems could be fix in one patch - I will leave it up to the > maintainers to make a call on two patches or a single patch. > > thanks, > -- Shuah >