[SCSI] NULL pointer dereference in sym53c8xx (bisected)

[SCSI] NULL pointer dereference in sym53c8xx (bisected)

[Stratos Psomadakis]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

Hi,

After upstream commit 4e6c82b3614a18740ef63109d58743a359266daf ([SCSI]
fix WARNING: at drivers/scsi/scsi_lib.c:1704), which is also included in
3.0-stable and 3.1-stable kernels, the kernel fails to boot (NULL
pointer dereference in sym53c8xx_slave_destroy).

Bug report at the Gentoo Bugzilla (reported and bisected by Martin von
Gagern). [1]  (stack trace [2])

I think that the problem is that (after commit 4e6c82b)
__scsi_remove_device() is called if slave_alloc() in scsi_alloc_sdev()
fails. But __scsi_remove_device() calls slave_destroy(), which (I think)
doesn't make much sense (ie to call slave_destroy() when slave_alloc()
fails).

For sym53c8xx, this results in a NULL pointer dereference (struct
sym_lcb pointer) in slave_destroy().

[1] https://bugs.gentoo.org/show_bug.cgi?id=392567
[2] https://392567.bugs.gentoo.org/attachment.cgi?id=294381

-- 
Stratos Psomadakis
<psomas@gentoo.org>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [SCSI] NULL pointer dereference in sym53c8xx (bisected)

[Steffen Maier]

On 12/02/2011 04:26 PM, Stratos Psomadakis wrote:
> After upstream commit 4e6c82b3614a18740ef63109d58743a359266daf ([SCSI]
> fix WARNING: at drivers/scsi/scsi_lib.c:1704), which is also included in
> 3.0-stable and 3.1-stable kernels, the kernel fails to boot (NULL
> pointer dereference in sym53c8xx_slave_destroy).
>
> Bug report at the Gentoo Bugzilla (reported and bisected by Martin von
> Gagern). [1]  (stack trace [2])
>
> I think that the problem is that (after commit 4e6c82b)
> __scsi_remove_device() is called if slave_alloc() in scsi_alloc_sdev()
> fails. But __scsi_remove_device() calls slave_destroy(), which (I think)
> doesn't make much sense (ie to call slave_destroy() when slave_alloc()
> fails).
>
> For sym53c8xx, this results in a NULL pointer dereference (struct
> sym_lcb pointer) in slave_destroy().
>
> [1] https://bugs.gentoo.org/show_bug.cgi?id=392567
> [2] https://392567.bugs.gentoo.org/attachment.cgi?id=294381

To me this looks like the same thing we encountered in the zfcp LLD:
http://www.spinics.net/lists/linux-scsi/msg55575.html
James explained the pairing of slave_alloc and slave_destroy even if 
slave_alloc returned early in which case slave_destroy needs to cope 
with that:
http://www.spinics.net/lists/linux-scsi/msg55449.html

HTH
Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [SCSI] NULL pointer dereference in sym53c8xx (bisected)

[Stratos Psomadakis]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

On 12/03/2011 11:07 PM, Steffen Maier wrote:
> On 12/02/2011 04:26 PM, Stratos Psomadakis wrote:
>> After upstream commit 4e6c82b3614a18740ef63109d58743a359266daf ([SCSI]
>> fix WARNING: at drivers/scsi/scsi_lib.c:1704), which is also included in
>> 3.0-stable and 3.1-stable kernels, the kernel fails to boot (NULL
>> pointer dereference in sym53c8xx_slave_destroy).
>>
>> Bug report at the Gentoo Bugzilla (reported and bisected by Martin von
>> Gagern). [1]  (stack trace [2])
>>
>> I think that the problem is that (after commit 4e6c82b)
>> __scsi_remove_device() is called if slave_alloc() in scsi_alloc_sdev()
>> fails. But __scsi_remove_device() calls slave_destroy(), which (I think)
>> doesn't make much sense (ie to call slave_destroy() when slave_alloc()
>> fails).
>>
>> For sym53c8xx, this results in a NULL pointer dereference (struct
>> sym_lcb pointer) in slave_destroy().
>>
>> [1] https://bugs.gentoo.org/show_bug.cgi?id=392567
>> [2] https://392567.bugs.gentoo.org/attachment.cgi?id=294381
>
> To me this looks like the same thing we encountered in the zfcp LLD:
> http://www.spinics.net/lists/linux-scsi/msg55575.html
> James explained the pairing of slave_alloc and slave_destroy even if
> slave_alloc returned early in which case slave_destroy needs to cope
> with that:
> http://www.spinics.net/lists/linux-scsi/msg55449.html
>
Indeed.

I'll follow-up with a patch similar to the one you sent for zfcp. I
think that returning if sym_lcb is NULL should be ok.

> HTH
> Steffen
>
> Linux on System z Development
>
> IBM Deutschland Research & Development GmbH
> Vorsitzender des Aufsichtsrats: Martin Jetter
> Geschäftsführung: Dirk Wittkopp
> Sitz der Gesellschaft: Böblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Stratos Psomadakis
<psomas@gentoo.org>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [SCSI] NULL pointer dereference in sym53c8xx (bisected)

[Stratos Psomadakis]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On 12/04/2011 02:11 AM, Stratos Psomadakis wrote:
> On 12/03/2011 11:07 PM, Steffen Maier wrote:
>> On 12/02/2011 04:26 PM, Stratos Psomadakis wrote:
>>> After upstream commit 4e6c82b3614a18740ef63109d58743a359266daf ([SCSI]
>>> fix WARNING: at drivers/scsi/scsi_lib.c:1704), which is also included in
>>> 3.0-stable and 3.1-stable kernels, the kernel fails to boot (NULL
>>> pointer dereference in sym53c8xx_slave_destroy).
>>>
>>> Bug report at the Gentoo Bugzilla (reported and bisected by Martin von
>>> Gagern). [1]  (stack trace [2])
>>>
>>> I think that the problem is that (after commit 4e6c82b)
>>> __scsi_remove_device() is called if slave_alloc() in scsi_alloc_sdev()
>>> fails. But __scsi_remove_device() calls slave_destroy(), which (I think)
>>> doesn't make much sense (ie to call slave_destroy() when slave_alloc()
>>> fails).
>>>
>>> For sym53c8xx, this results in a NULL pointer dereference (struct
>>> sym_lcb pointer) in slave_destroy().
>>>
>>> [1] https://bugs.gentoo.org/show_bug.cgi?id=392567
>>> [2] https://392567.bugs.gentoo.org/attachment.cgi?id=294381
>> To me this looks like the same thing we encountered in the zfcp LLD:
>> http://www.spinics.net/lists/linux-scsi/msg55575.html
>> James explained the pairing of slave_alloc and slave_destroy even if
>> slave_alloc returned early in which case slave_destroy needs to cope
>> with that:
>> http://www.spinics.net/lists/linux-scsi/msg55449.html
>>
> Indeed.
>
> I'll follow-up with a patch similar to the one you sent for zfcp. I
> think that returning if sym_lcb is NULL should be ok.
I forgot to chain the patch email in this thread, so here's the link:
http://marc.info/?l=linux-scsi&m=132295936832641&w=2

>> HTH
>> Steffen
>>
>> Linux on System z Development
>>
>> IBM Deutschland Research & Development GmbH
>> Vorsitzender des Aufsichtsrats: Martin Jetter
>> Geschäftsführung: Dirk Wittkopp
>> Sitz der Gesellschaft: Böblingen
>> Registergericht: Amtsgericht Stuttgart, HRB 243294
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Stratos Psomadakis
<psomas@gentoo.org>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]