* Re: [PATCH v2] scsi/cxgbi/libcxgbi: make sure sg is present before calling sg_next() [not found] <20221114150923.3544796-1-d-tatianin@yandex-team.ru> @ 2022-11-14 22:23 ` Mike Christie [not found] ` <84a02b26-392e-994d-bd1f-ecc28231fbfc@yandex-team.ru> 0 siblings, 1 reply; 3+ messages in thread From: Mike Christie @ 2022-11-14 22:23 UTC (permalink / raw) To: Daniil Tatianin, James E.J. Bottomley Cc: Martin K. Petersen, Lee Duncan, Nilesh Javali, Wu Bo, linux-scsi, linux-kernel, lvc-project, yc-core On 11/14/22 9:09 AM, Daniil Tatianin wrote: > sg_next() dereferences the passed sg, therefore we have to verify that > it's present before calling it. > > Found by Linux Verification Center (linuxtesting.org) with the SVACE > static analysis tool. > > Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> > --- > drivers/scsi/cxgbi/libcxgbi.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c > index af281e271f88..2ff9810f42a9 100644 > --- a/drivers/scsi/cxgbi/libcxgbi.c > +++ b/drivers/scsi/cxgbi/libcxgbi.c > @@ -1196,8 +1196,7 @@ void cxgbi_ddp_set_one_ppod(struct cxgbi_pagepod *ppod, > > if (offset == len) { > offset = 0; > - sg = sg_next(sg); > - if (sg) { > + if (sg && (sg = sg_next(sg))) { > addr = sg_dma_address(sg); > len = sg_dma_len(sg); > } Is cxgbit_set_one_ppod the same function but it already has the extra sg check? Should it be a libcxgb function in libcxgb_ppm.c? ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <84a02b26-392e-994d-bd1f-ecc28231fbfc@yandex-team.ru>]
* Re: [PATCH v2] scsi/cxgbi/libcxgbi: make sure sg is present before calling sg_next() [not found] ` <84a02b26-392e-994d-bd1f-ecc28231fbfc@yandex-team.ru> @ 2022-11-15 16:59 ` Mike Christie [not found] ` <20075199-1e6c-28ad-1440-4dc61985d8f4@yandex-team.ru> 0 siblings, 1 reply; 3+ messages in thread From: Mike Christie @ 2022-11-15 16:59 UTC (permalink / raw) To: Daniil Tatianin, James E.J. Bottomley Cc: Martin K. Petersen, Lee Duncan, Nilesh Javali, Wu Bo, linux-scsi, linux-kernel, lvc-project, yc-core, Varun Prakash Cc'ing the cxgbi/t maintainer, Varun. On 11/15/22 2:17 AM, Daniil Tatianin wrote: > On 11/15/22 1:23 AM, Mike Christie wrote: >> On 11/14/22 9:09 AM, Daniil Tatianin wrote: >>> sg_next() dereferences the passed sg, therefore we have to verify that >>> it's present before calling it. >>> >>> Found by Linux Verification Center (linuxtesting.org) with the SVACE >>> static analysis tool. >>> >>> Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru> >>> --- >>> drivers/scsi/cxgbi/libcxgbi.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c >>> index af281e271f88..2ff9810f42a9 100644 >>> --- a/drivers/scsi/cxgbi/libcxgbi.c >>> +++ b/drivers/scsi/cxgbi/libcxgbi.c >>> @@ -1196,8 +1196,7 @@ void cxgbi_ddp_set_one_ppod(struct cxgbi_pagepod *ppod, >>> if (offset == len) { >>> offset = 0; >>> - sg = sg_next(sg); >>> - if (sg) { >>> + if (sg && (sg = sg_next(sg))) { >>> addr = sg_dma_address(sg); >>> len = sg_dma_len(sg); >>> } >> >> Is cxgbit_set_one_ppod the same function but it already has the extra >> sg check? > > Good catch! Certainly looks that way, albeit with messier indentation. > >> Should it be a libcxgb function in libcxgb_ppm.c? > > That makes sense to me. Should I just move both there? I think you can move one function with a fix to libcxgb and kill the second one. Name the new function to cxgb_ddp_set_one_ppod then have cxgbi and cxgbt use it. ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <20075199-1e6c-28ad-1440-4dc61985d8f4@yandex-team.ru>]
* RE: [PATCH v2] scsi/cxgbi/libcxgbi: make sure sg is present before calling sg_next() [not found] ` <20075199-1e6c-28ad-1440-4dc61985d8f4@yandex-team.ru> @ 2022-11-17 13:30 ` Varun Prakash 0 siblings, 0 replies; 3+ messages in thread From: Varun Prakash @ 2022-11-17 13:30 UTC (permalink / raw) To: Daniil Tatianin, Mike Christie, James E.J. Bottomley Cc: Martin K. Petersen, Lee Duncan, Nilesh Javali, Wu Bo, linux-scsi, linux-kernel, lvc-project, yc-core >>>>> diff --git a/drivers/scsi/cxgbi/libcxgbi.c >>>>> b/drivers/scsi/cxgbi/libcxgbi.c index af281e271f88..2ff9810f42a9 >>>>> 100644 >>>>> --- a/drivers/scsi/cxgbi/libcxgbi.c >>>>> +++ b/drivers/scsi/cxgbi/libcxgbi.c >>>>> @@ -1196,8 +1196,7 @@ void cxgbi_ddp_set_one_ppod(struct >>>>> cxgbi_pagepod *ppod, >>>>> if (offset == len) { >>>>> offset = 0; >>>>> - sg = sg_next(sg); >>>>> - if (sg) { >>>>> + if (sg && (sg = sg_next(sg))) { >>>>> addr = sg_dma_address(sg); >>>>> len = sg_dma_len(sg); >>>>> } >>>> >>>> Is cxgbit_set_one_ppod the same function but it already has the >>>> extra sg check? >>> >>> Good catch! Certainly looks that way, albeit with messier indentation. >>> >>>> Should it be a libcxgb function in libcxgb_ppm.c? >>> >>> That makes sense to me. Should I just move both there? >> >> I think you can move one function with a fix to libcxgb and kill the second one. >> Name the new function to cxgb_ddp_set_one_ppod then have cxgbi and cxgbt use it. > >Yeah, thah's pretty much what I meant. Thank you! Yes, you can move cxgbit_set_one_ppod() from cxgbit_ddp.c to libcxgb_ppm.c and rename it to cxgbi_ppm_set_one_ppod() and remove cxgbi_ddp_set_one_ppod() from libcxgbi.c. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-17 13:31 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20221114150923.3544796-1-d-tatianin@yandex-team.ru> 2022-11-14 22:23 ` [PATCH v2] scsi/cxgbi/libcxgbi: make sure sg is present before calling sg_next() Mike Christie [not found] ` <84a02b26-392e-994d-bd1f-ecc28231fbfc@yandex-team.ru> 2022-11-15 16:59 ` Mike Christie [not found] ` <20075199-1e6c-28ad-1440-4dc61985d8f4@yandex-team.ru> 2022-11-17 13:30 ` Varun Prakash
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.