Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/13/2016 06:24 PM, Johannes Thumshirn wrote:
On Thu, Oct 13, 2016 at 05:15:25PM +0200, Steffen Maier wrote:
I'm puzzled.

$ git bisect start fc_bsg master

3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit
commit 3087864ce3d7282f59021245d8a5f83ef1caef18
Author: Johannes Thumshirn <jthumshirn@xxxxxxx>
Date:   Wed Oct 12 15:06:28 2016 +0200

    scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly

    Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but use
    helper variables bsg_request and bsg_reply. This will be helpfull  when
    transitioning to bsg-lib.

    Signed-off-by: Johannes Thumshirn <jthumshirn@xxxxxxx>

:040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe225615679550be91fbd9f84c09ab1e280fc M	drivers

From there (on the reverse bisect path) I get the following Oops,
except for the full patch set having another stack trace as in my previous
mail (dying in zfcp code).


[...]


@@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
 	struct request *req;
 	struct fc_bsg_job *job;
 	enum fc_dispatch_result ret;
+	struct fc_bsg_reply *bsg_reply;

 	if (!get_device(dev))
 		return;
@@ -3973,8 +3981,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
 		/* check if we have the msgcode value at least */
 		if (job->request_len < sizeof(uint32_t)) {
 			BUG_ON(job->reply_len < sizeof(uint32_t));
-			job->reply->reply_payload_rcv_len = 0;
-			job->reply->result = -ENOMSG;
+			bsg_reply = job->reply;
+			bsg_reply->reply_payload_rcv_len = 0;
+			bsg_reply->result = -ENOMSG;

Compiler optimization re-ordered above two lines and the first pointer derefence is bsg_reply->result [field offset 0] where bsg_reply is NULL. The assignment tries to write to memory at address NULL causing the kernel page fault.

Does your suggested change for [PATCH v3 02/16], shuffling the job->request_len checks, address above kernel page fault?

 			job->reply_len = sizeof(uint32_t);
 			fc_bsg_jobdone(job);
 			spin_lock_irq(q->queue_lock);


Ahm and what exactly can break here? It's just assigning variables. Now
I'm puzzled too.


--
Mit freundlichen Grüßen / Kind regards
Steffen Maier

Linux on z Systems Development

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschaeftsfuehrung: Dirk Wittkopp
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux