Messages in this thread | | | Subject | Re: [PATCH v2 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly | From | Steffen Maier <> | Date | Fri, 28 Oct 2016 11:53:46 +0200 |
| |
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@suse.de> >>> 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@suse.de> >>> >>> :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
| |