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

Previous thread: [GIT PULL] 9p file system bug fixes for 2.6.36-rc4 by Eric Van Hensbergen on Monday, August 30, 2010 - 10:54 am. (1 message)

Next thread: [PATCH] HID: roccat: Normalized reported profile number for pyra button events. by Stefan Achatz on Monday, August 30, 2010 - 12:28 pm. (2 messages)
From: Alexey Dobriyan
Date: Monday, August 30, 2010 - 11:46 am

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

From: Linus Torvalds
Date: Wednesday, September 8, 2010 - 5:12 pm

Hmm. No noise about this one.

Jens, Fujita, James, any comments?


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

From: Jens Axboe
Date: Thursday, September 9, 2010 - 4:00 am

Agree, that's clearly a bug. That assignment should just go away.

-- 
Jens Axboe

--

From: James Bottomley
Date: Thursday, September 9, 2010 - 5:47 am

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


--

From: Jens Axboe
Date: Thursday, September 9, 2010 - 5:51 am

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

--

From: James Bottomley
Date: Thursday, September 9, 2010 - 10:30 am

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



--

Previous thread: [GIT PULL] 9p file system bug fixes for 2.6.36-rc4 by Eric Van Hensbergen on Monday, August 30, 2010 - 10:54 am. (1 message)

Next thread: [PATCH] HID: roccat: Normalized reported profile number for pyra button events. by Stefan Achatz on Monday, August 30, 2010 - 12:28 pm. (2 messages)