* 2.6.36-rc3: EIP is at scsi_init_io+...
@ 2010-08-30 18:46 Alexey Dobriyan
2010-09-09 0:12 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Alexey Dobriyan @ 2010-08-30 18:46 UTC (permalink / raw)
To: torvalds, akpm, James.Bottomley; +Cc: linux-scsi, linux-kernel
Not much of a calltrace, it scrolled away because of hardlockup detector.
On the bright side, radeon KMS worked correctly and actually showed it.
$ addr2line -e vmlinux ffffffff812d207b
drivers/scsi/scsi_lib.c:1015
969 int scsi_init_io(struct scsi_cmnd *cmd, gfp_t gfp_mask)
970 {
971 int error = scsi_init_sgtable(cmd->request, &cmd->sdb, gfp_mask);
972 if (error)
973 goto err_exit;
974
975 if (blk_bidi_rq(cmd->request)) {
976 struct scsi_data_buffer *bidi_sdb = kmem_cache_zalloc(
977 scsi_sdb_cache, GFP_ATOMIC);
978 if (!bidi_sdb) {
979 error = BLKPREP_DEFER;
980 goto err_exit;
981 }
982
983 cmd->request->next_rq->special = bidi_sdb;
984 error = scsi_init_sgtable(cmd->request->next_rq, bidi_sdb,
985 GFP_ATOMIC);
986 if (error)
987 goto err_exit;
988 }
989
990 if (blk_integrity_rq(cmd->request)) {
991 struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
992 int ivecs, count;
993
994 BUG_ON(prot_sdb == NULL);
995 ivecs = blk_rq_count_integrity_sg(cmd->request);
996
997 if (scsi_alloc_sgtable(prot_sdb, ivecs, gfp_mask)) {
998 error = BLKPREP_DEFER;
999 goto err_exit;
1000 }
1001
1002 count = blk_rq_map_integrity_sg(cmd->request,
1003 prot_sdb->table.sgl);
1004 BUG_ON(unlikely(count > ivecs));
1005
1006 cmd->prot_sdb = prot_sdb;
1007 cmd->prot_sdb->table.nents = count;
1008 }
1009
1010 return BLKPREP_OK ;
1011
1012 err_exit:
1013 scsi_release_buffers(cmd);
1014 scsi_put_command(cmd);
1015 ===> cmd->request->special = NULL; <===
1016 return error;
1017 }
1018 EXPORT_SYMBOL(scsi_init_io);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 2.6.36-rc3: EIP is at scsi_init_io+...
2010-08-30 18:46 2.6.36-rc3: EIP is at scsi_init_io+ Alexey Dobriyan
@ 2010-09-09 0:12 ` Linus Torvalds
2010-09-09 11:00 ` Jens Axboe
2010-09-09 12:47 ` James Bottomley
0 siblings, 2 replies; 6+ messages in thread
From: Linus Torvalds @ 2010-09-09 0:12 UTC (permalink / raw)
To: Alexey Dobriyan, FUJITA Tomonori, Jens Axboe
Cc: akpm, James.Bottomley, linux-scsi, linux-kernel
Hmm. No noise about this one.
Jens, Fujita, James, any comments?
On Mon, Aug 30, 2010 at 11:46 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> Not much of a calltrace, it scrolled away because of hardlockup detector.
> On the bright side, radeon KMS worked correctly and actually showed it.
>
> $ addr2line -e vmlinux ffffffff812d207b
> drivers/scsi/scsi_lib.c:1015
>
> 1012 err_exit:
> 1013 scsi_release_buffers(cmd);
> 1014 scsi_put_command(cmd);
> 1015 ===> cmd->request->special = NULL; <===
> 1016 return error;
> 1017 }
> 1018 EXPORT_SYMBOL(scsi_init_io);
I do have to say that it looks rather wrong that it accesses "cmd"
after it has done the "scsi_put_command(cmd)" on it.
I also note that that was introduced pretty recently by commit
610a63498f7 ("scsi: fix discard page leak"), merged during this merge
window. That does look suspicious to me.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 2.6.36-rc3: EIP is at scsi_init_io+...
2010-09-09 0:12 ` Linus Torvalds
@ 2010-09-09 11:00 ` Jens Axboe
2010-09-09 12:47 ` James Bottomley
1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2010-09-09 11:00 UTC (permalink / raw)
To: Linus Torvalds
Cc: Alexey Dobriyan, FUJITA Tomonori, akpm@linux-foundation.org,
James.Bottomley@suse.de, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
On 2010-09-09 02:12, Linus Torvalds wrote:
> Hmm. No noise about this one.
>
> Jens, Fujita, James, any comments?
>
> On Mon, Aug 30, 2010 at 11:46 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>> Not much of a calltrace, it scrolled away because of hardlockup detector.
>> On the bright side, radeon KMS worked correctly and actually showed it.
>>
>> $ addr2line -e vmlinux ffffffff812d207b
>> drivers/scsi/scsi_lib.c:1015
>>
>> 1012 err_exit:
>> 1013 scsi_release_buffers(cmd);
>> 1014 scsi_put_command(cmd);
>> 1015 ===> cmd->request->special = NULL; <===
>> 1016 return error;
>> 1017 }
>> 1018 EXPORT_SYMBOL(scsi_init_io);
>
> I do have to say that it looks rather wrong that it accesses "cmd"
> after it has done the "scsi_put_command(cmd)" on it.
>
> I also note that that was introduced pretty recently by commit
> 610a63498f7 ("scsi: fix discard page leak"), merged during this merge
> window. That does look suspicious to me.
Agree, that's clearly a bug. That assignment should just go away.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 2.6.36-rc3: EIP is at scsi_init_io+...
2010-09-09 0:12 ` Linus Torvalds
2010-09-09 11:00 ` Jens Axboe
@ 2010-09-09 12:47 ` James Bottomley
2010-09-09 12:51 ` Jens Axboe
1 sibling, 1 reply; 6+ messages in thread
From: James Bottomley @ 2010-09-09 12:47 UTC (permalink / raw)
To: Linus Torvalds
Cc: Alexey Dobriyan, FUJITA Tomonori, Jens Axboe, akpm, linux-scsi,
linux-kernel
On Wed, 2010-09-08 at 17:12 -0700, Linus Torvalds wrote:
> Hmm. No noise about this one.
>
> Jens, Fujita, James, any comments?
>
> On Mon, Aug 30, 2010 at 11:46 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> > Not much of a calltrace, it scrolled away because of hardlockup detector.
> > On the bright side, radeon KMS worked correctly and actually showed it.
> >
> > $ addr2line -e vmlinux ffffffff812d207b
> > drivers/scsi/scsi_lib.c:1015
> >
> > 1012 err_exit:
> > 1013 scsi_release_buffers(cmd);
> > 1014 scsi_put_command(cmd);
> > 1015 ===> cmd->request->special = NULL; <===
> > 1016 return error;
> > 1017 }
> > 1018 EXPORT_SYMBOL(scsi_init_io);
>
> I do have to say that it looks rather wrong that it accesses "cmd"
> after it has done the "scsi_put_command(cmd)" on it.
>
> I also note that that was introduced pretty recently by commit
> 610a63498f7 ("scsi: fix discard page leak"), merged during this merge
> window. That does look suspicious to me.
It's a use after free: The put actually frees the cmnd and then we use
it to get to the request. Most of the time nothing notices, but if you
have poison on free enabled, we may see the problem. The fix is just to
reverse the put and the set.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 2.6.36-rc3: EIP is at scsi_init_io+...
2010-09-09 12:47 ` James Bottomley
@ 2010-09-09 12:51 ` Jens Axboe
2010-09-09 17:30 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2010-09-09 12:51 UTC (permalink / raw)
To: James Bottomley
Cc: Linus Torvalds, Alexey Dobriyan, FUJITA Tomonori,
akpm@linux-foundation.org, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
On 2010-09-09 14:47, James Bottomley wrote:
> On Wed, 2010-09-08 at 17:12 -0700, Linus Torvalds wrote:
>> Hmm. No noise about this one.
>>
>> Jens, Fujita, James, any comments?
>>
>> On Mon, Aug 30, 2010 at 11:46 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
>>> Not much of a calltrace, it scrolled away because of hardlockup detector.
>>> On the bright side, radeon KMS worked correctly and actually showed it.
>>>
>>> $ addr2line -e vmlinux ffffffff812d207b
>>> drivers/scsi/scsi_lib.c:1015
>>>
>>> 1012 err_exit:
>>> 1013 scsi_release_buffers(cmd);
>>> 1014 scsi_put_command(cmd);
>>> 1015 ===> cmd->request->special = NULL; <===
>>> 1016 return error;
>>> 1017 }
>>> 1018 EXPORT_SYMBOL(scsi_init_io);
>>
>> I do have to say that it looks rather wrong that it accesses "cmd"
>> after it has done the "scsi_put_command(cmd)" on it.
>>
>> I also note that that was introduced pretty recently by commit
>> 610a63498f7 ("scsi: fix discard page leak"), merged during this merge
>> window. That does look suspicious to me.
>
> It's a use after free: The put actually frees the cmnd and then we use
> it to get to the request. Most of the time nothing notices, but if you
> have poison on free enabled, we may see the problem. The fix is just to
> reverse the put and the set.
You are right, I misspoke in my original reply. It's clearing the
request field, not the command field (which would be bogus of course).
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: 2.6.36-rc3: EIP is at scsi_init_io+...
2010-09-09 12:51 ` Jens Axboe
@ 2010-09-09 17:30 ` James Bottomley
0 siblings, 0 replies; 6+ messages in thread
From: James Bottomley @ 2010-09-09 17:30 UTC (permalink / raw)
To: Jens Axboe
Cc: Linus Torvalds, Alexey Dobriyan, FUJITA Tomonori,
akpm@linux-foundation.org, linux-scsi@vger.kernel.org,
linux-kernel@vger.kernel.org
On Thu, 2010-09-09 at 14:51 +0200, Jens Axboe wrote:
> On 2010-09-09 14:47, James Bottomley wrote:
> > On Wed, 2010-09-08 at 17:12 -0700, Linus Torvalds wrote:
> >> Hmm. No noise about this one.
> >>
> >> Jens, Fujita, James, any comments?
> >>
> >> On Mon, Aug 30, 2010 at 11:46 AM, Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >>> Not much of a calltrace, it scrolled away because of hardlockup detector.
> >>> On the bright side, radeon KMS worked correctly and actually showed it.
> >>>
> >>> $ addr2line -e vmlinux ffffffff812d207b
> >>> drivers/scsi/scsi_lib.c:1015
> >>>
> >>> 1012 err_exit:
> >>> 1013 scsi_release_buffers(cmd);
> >>> 1014 scsi_put_command(cmd);
> >>> 1015 ===> cmd->request->special = NULL; <===
> >>> 1016 return error;
> >>> 1017 }
> >>> 1018 EXPORT_SYMBOL(scsi_init_io);
> >>
> >> I do have to say that it looks rather wrong that it accesses "cmd"
> >> after it has done the "scsi_put_command(cmd)" on it.
> >>
> >> I also note that that was introduced pretty recently by commit
> >> 610a63498f7 ("scsi: fix discard page leak"), merged during this merge
> >> window. That does look suspicious to me.
> >
> > It's a use after free: The put actually frees the cmnd and then we use
> > it to get to the request. Most of the time nothing notices, but if you
> > have poison on free enabled, we may see the problem. The fix is just to
> > reverse the put and the set.
>
> You are right, I misspoke in my original reply. It's clearing the
> request field, not the command field (which would be bogus of course).
Embarrassingly enough, Tejun spotted this a month ago, and I actually
created a patch and then forgot to apply it. I've just pushed it out to
the rc-fixes branch:
http://marc.info/?l=linux-scsi&m=128197119525504
James
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-09 17:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-08-30 18:46 2.6.36-rc3: EIP is at scsi_init_io+ Alexey Dobriyan
2010-09-09 0:12 ` Linus Torvalds
2010-09-09 11:00 ` Jens Axboe
2010-09-09 12:47 ` James Bottomley
2010-09-09 12:51 ` Jens Axboe
2010-09-09 17:30 ` James Bottomley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).