Thread (43 messages) 43 messages, 4 authors, 2016-10-30

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

From: Hannes Reinecke <hare@suse.de>
Date: 2016-10-28 11:31:24
Also in: linux-scsi, linuxppc-dev

On 10/28/2016 11:53 AM, Steffen Maier wrote:

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

$ git bisect start fc_bsg master
quoted
quoted
quoted
3087864ce3d7282f59021245d8a5f83ef1caef18 is the first bad commit
commit 3087864ce3d7282f59021245d8a5f83ef1caef18
Author: Johannes Thumshirn [off-list ref]
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 [off-list ref]

: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).
[...]
quoted
quoted
@@ -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.
I spoke to our compiler people, and they strongly believed this not to
be the case. Or, put it the other way round, if such a thing would
happen it would be a compiler issue.

Have you checked the compiler output?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help