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.
