Re: [PATCH] scsi: sni_53c710: Fix a resource leak in an error handling path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 20/05/2021 à 17:06, Thomas Bogendoerfer a écrit :
On Thu, May 20, 2021 at 06:44:25AM +0200, Christophe JAILLET wrote:
After a successful 'NCR_700_detect()' call, 'NCR_700_release()' must be
called to release some DMA related resources, as already done in the
remove function.

Fixes: c27d85f3f3c5 ("[SCSI] SNI RM 53c710 driver")
Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
---
  drivers/scsi/sni_53c710.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/sni_53c710.c b/drivers/scsi/sni_53c710.c
index 678651b9b4dd..f6d60d542207 100644
--- a/drivers/scsi/sni_53c710.c
+++ b/drivers/scsi/sni_53c710.c
@@ -98,6 +98,7 @@ static int snirm710_probe(struct platform_device *dev)
out_put_host:
  	scsi_host_put(host);
+	NCR_700_release(host);

shouldn't this done before the scsi_host_put() to avoid a use after free ?

I did it this way because remove function are:
   scsi_remove_host
   NCR_700_release

The other reason was to free resources in the reverse order than allocation.
But this logic does'nt work in all cases.

lasi700.c has the same problem.

and a400t.c and bvme6000_scsi.c and mvme16x_scsi.c and sim710.c and zorro7xx.c.
That is to say all drivers that use NCR_700_detect().

That is why I'm unsure with my patch. It is unusual to have ALL users wrong. It is more likely that I missed something and that the code is correct.

I also don't fully understand why we use 'scsi_host_put' in some place and 'scsi_remove_host' in remove functions. Referenced managed resources sometimes have the needed mechanism to automagically release some resource.

And it looks like NCR_700_detect() will leak
dma memory, if scsi_host_alloc() failed.

Agreed. And same if scsi_add_host fails at the end of the function.


Thomas.





[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux