On 6/9/20 2:04 PM, Ben Hutchings wrote:
3.16.85-rc1 review patch. If anyone has any objections, please let me know.
From: Johannes Thumshirn jthumshirn@suse.de
commit 48ae8484e9fc324b4968d33c585e54bc98e44d61 upstream.
If the list search in sg_get_rq_mark() fails to find a valid request, we return a bogus element. This then can later lead to a GPF in sg_remove_scat().
So don't return bogus Sg_requests in sg_get_rq_mark() but NULL in case the list search doesn't find a valid request.
Signed-off-by: Johannes Thumshirn jthumshirn@suse.de Reported-by: Andrey Konovalov andreyknvl@google.com Cc: Hannes Reinecke hare@suse.de Cc: Christoph Hellwig hch@lst.de Cc: Doug Gilbert dgilbert@interlog.com Reviewed-by: Hannes Reinecke hare@suse.de Acked-by: Doug Gilbert dgilbert@interlog.com Signed-off-by: Martin K. Petersen martin.petersen@oracle.com Cc: Tony Battersby tonyb@cybernetics.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Ben Hutchings ben@decadent.org.uk
drivers/scsi/sg.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
--- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -2085,11 +2085,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 */
break;
write_unlock_irqrestore(&sfp->rq_list_lock, iflags);return resp;
- return resp;
- return NULL;
} /* always adds to end of list */
The following "cleanup" commit to the sg driver introduced a number of bugs:
109bade9c625 ("scsi: sg: use standard lists for sg_requests") (v4.12-rc1)
This one bad commit requires all of the following fixes:
48ae8484e9fc ("scsi: sg: don't return bogus Sg_requests") (v4.12-rc1) bd46fc406b30 ("scsi: sg: off by one in sg_ioctl()") (v4.13-rc7) 4759df905a47 ("scsi: sg: factor out sg_fill_request_table()") (v4.14-rc1) 3e0097499839 ("scsi: sg: fixup infoleak when using SG_GET_REQUEST_TABLE") (v4.14-rc1) 587c3c9f286c ("scsi: sg: Re-fix off by one in sg_fill_request_table()") (v4.14-rc6)
AFAIK, there is no reason to backport any of these changes to -stable, but if for some reason you do need to backport any one of these patches, then make sure you get all of them.
My guess is that the initial buggy patch was backported to other -stable trees because the fixes for it looked important, and of course the fixes depended on the patch that introduced all of the problems to begin with.
Tony Battersby Cybernetics