* [PATCH] scsi: sg: Fix mismatch in sg_get_rq_mark
@ 2017-05-16 14:52 firogm
2017-05-16 14:55 ` Andrey Konovalov
0 siblings, 1 reply; 2+ messages in thread
From: firogm @ 2017-05-16 14:52 UTC (permalink / raw)
To: dgilbert
Cc: dvyukov, jejb, martin.petersen, linux-scsi, syzkaller,
linux-kernel, Firo Yang
From: Firo Yang <firogm@gmail.com>
This bug was reported by Andrey Konovalov with syzkaller:
Call Trace:
sg_finish_rem_req+0x2a6/0x320 drivers/scsi/sg.c:1839
sg_new_read+0x3c/0x430 drivers/scsi/sg.c:567
sg_read+0x866/0x1830 drivers/scsi/sg.c:455
__vfs_read+0x5c3/0x750 fs/read_write.c:450
vfs_read+0x11e/0x350 fs/read_write.c:473
SYSC_read fs/read_write.c:589
SyS_read+0xfb/0x230 fs/read_write.c:582
entry_SYSCALL_64_fastpath+0x1f/0xbe arch/x86/entry/entry_64.S:204
If there is nothing matched in the loop of list_for_each_entry,
at last the iteration variable 'resp' will point to
list_entry(&sfp->rq_list, typeof(resp), entry); it's not a valid
Sg_request. Finnally, 'resp' will be wrongly returned to the caller
function.
To fix it, add another return variable to record the correct
match and return it to the caller function.
Signed-off-by: Firo Yang <firogm@gmail.com>
---
drivers/scsi/sg.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0a38ba0..74a9188 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2065,7 +2065,7 @@ sg_unlink_reserve(Sg_fd * sfp, Sg_request * srp)
static Sg_request *
sg_get_rq_mark(Sg_fd * sfp, int pack_id)
{
- Sg_request *resp;
+ Sg_request *resp, *ret_resp = NULL;
unsigned long iflags;
write_lock_irqsave(&sfp->rq_list_lock, iflags);
@@ -2074,11 +2074,12 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
if ((1 == resp->done) && (!resp->sg_io_owned) &&
((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
resp->done = 2; /* guard against other readers */
+ ret_resp = resp;
break;
}
}
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
- return resp;
+ return ret_resp;
}
/* always adds to end of list */
--
2.9.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] scsi: sg: Fix mismatch in sg_get_rq_mark
2017-05-16 14:52 [PATCH] scsi: sg: Fix mismatch in sg_get_rq_mark firogm
@ 2017-05-16 14:55 ` Andrey Konovalov
0 siblings, 0 replies; 2+ messages in thread
From: Andrey Konovalov @ 2017-05-16 14:55 UTC (permalink / raw)
To: firogm
Cc: Doug Gilbert, Dmitry Vyukov, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, syzkaller, LKML
On Tue, May 16, 2017 at 4:52 PM, <firogm@gmail.com> wrote:
> From: Firo Yang <firogm@gmail.com>
>
> This bug was reported by Andrey Konovalov with syzkaller:
>
> Call Trace:
> sg_finish_rem_req+0x2a6/0x320 drivers/scsi/sg.c:1839
> sg_new_read+0x3c/0x430 drivers/scsi/sg.c:567
> sg_read+0x866/0x1830 drivers/scsi/sg.c:455
> __vfs_read+0x5c3/0x750 fs/read_write.c:450
> vfs_read+0x11e/0x350 fs/read_write.c:473
> SYSC_read fs/read_write.c:589
> SyS_read+0xfb/0x230 fs/read_write.c:582
> entry_SYSCALL_64_fastpath+0x1f/0xbe arch/x86/entry/entry_64.S:204
>
> If there is nothing matched in the loop of list_for_each_entry,
> at last the iteration variable 'resp' will point to
> list_entry(&sfp->rq_list, typeof(resp), entry); it's not a valid
> Sg_request. Finnally, 'resp' will be wrongly returned to the caller
> function.
>
> To fix it, add another return variable to record the correct
> match and return it to the caller function.
I think this was already fixed by https://lkml.org/lkml/2017/5/10/83
Thanks!
>
> Signed-off-by: Firo Yang <firogm@gmail.com>
> ---
> drivers/scsi/sg.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index 0a38ba0..74a9188 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -2065,7 +2065,7 @@ sg_unlink_reserve(Sg_fd * sfp, Sg_request * srp)
> static Sg_request *
> sg_get_rq_mark(Sg_fd * sfp, int pack_id)
> {
> - Sg_request *resp;
> + Sg_request *resp, *ret_resp = NULL;
> unsigned long iflags;
>
> write_lock_irqsave(&sfp->rq_list_lock, iflags);
> @@ -2074,11 +2074,12 @@ sg_get_rq_mark(Sg_fd * sfp, int pack_id)
> if ((1 == resp->done) && (!resp->sg_io_owned) &&
> ((-1 == pack_id) || (resp->header.pack_id == pack_id))) {
> resp->done = 2; /* guard against other readers */
> + ret_resp = resp;
> break;
> }
> }
> write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
> - return resp;
> + return ret_resp;
> }
>
> /* always adds to end of list */
> --
> 2.9.4
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-05-16 14:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 14:52 [PATCH] scsi: sg: Fix mismatch in sg_get_rq_mark firogm
2017-05-16 14:55 ` Andrey Konovalov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.