Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
From: Steffen Maier <hidden>
Date: 2016-10-28 09:55:03
Also in:
linux-scsi, lkml
On 10/13/2016 06:24 PM, Johannes Thumshirn wrote:
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
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 directl=
y
quoted
quoted
Don't use fc_bsg_job::request and fc_bsg_job::reply directly, but=
use
quoted
quoted
helper variables bsg_request and bsg_reply. This will be helpfull=
when
quoted
quoted
transitioning to bsg-lib. Signed-off-by: Johannes Thumshirn [off-list ref] :040000 040000 140c4b6829d5cfaec4079716e0795f63f8bc3bd2 0d9fe22561567=
9550be91fbd9f84c09ab1e280fc M drivers
quoted
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 prev=
ious
quoted
mail (dying in zfcp code).[...]quoted
quoted
@@ -3937,6 +3944,7 @@ fc_bsg_request_handler(struct request_queue *q,=
struct Scsi_Host *shost,
quoted
quoted
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,
quoted
quoted
/* 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 =3D 0; - job->reply->result =3D -ENOMSG; + bsg_reply =3D job->reply; + bsg_reply->reply_payload_rcv_len =3D 0; + bsg_reply->result =3D -ENOMSG;
Compiler optimization re-ordered above two lines and the first pointer=20 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=20 kernel page fault. Does your suggested change for [PATCH v3 02/16], shuffling the=20 job->request_len checks, address above kernel page fault?
quoted
quoted
job->reply_len =3D 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.
--=20 Mit freundlichen Gr=FC=DFen / 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