Skip to contentSkip to conversation

2.6.36-rc3: EIP is at scsi_init_io+...

DISCUSSION Alexey Dobriyan 4 participants 6 messages Aug 30, 2010 – Sep 9, 2010 SCSI

lore
Share & tools
lore

Alexey Dobriyan

author
#0 lore raw

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);
#1 lore raw

Hmm. No noise about this one.

Jens, Fujita, James, any comments?

Quoted 6 lines · Alexey Dobriyan

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
Quoted 7 lines

 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 19 lines · Linus Torvalds

Hmm. No noise about this one.

Jens, Fujita, James, any comments?

Quoted 6 lines · Alexey Dobriyan

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
Quoted 7 lines

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);

Quoted 6 lines

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.

Signature · 1 line
Jens Axboe
#3 lore raw
In reply to Linus Torvalds
Quoted 19 lines · Linus Torvalds

Hmm. No noise about this one.

Jens, Fujita, James, any comments?

Quoted 6 lines · Alexey Dobriyan

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
Quoted 7 lines

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);

Quoted 6 lines

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

#4 lore raw
Quoted 23 lines · James Bottomley
Quoted 22 lines · Linus Torvalds

Hmm. No noise about this one.

Jens, Fujita, James, any comments?

Quoted 6 lines · Alexey Dobriyan

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
Quoted 7 lines

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.

Quoted 8 lines
Quoted 3 lines

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).

Signature · 1 line
Jens Axboe
#5 lore raw
Quoted 28 lines · Jens Axboe
Quoted 27 lines · James Bottomley
Quoted 25 lines · Linus Torvalds

Hmm. No noise about this one.

Jens, Fujita, James, any comments?

Quoted 6 lines · Alexey Dobriyan

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
Quoted 7 lines

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.

Quoted 7 lines
Quoted 4 lines

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