* [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops
@ 2015-01-28 9:46 michaelc
2015-01-29 8:45 ` Hannes Reinecke
0 siblings, 1 reply; 4+ messages in thread
From: michaelc @ 2015-01-28 9:46 UTC (permalink / raw)
To: linux-scsi
From: Mike Christie <michaelc@cs.wisc.edu>
This fixes a regression caused by commit
1d5203284d8acbdfdf9b478d434450b34f338f28
The bug is that the alua detach() callout will try to access the
sddev->scsi_dh_data, but we have already set it to NULL. This patch
moves the clearing of that field to after detach() is called.
It looks like the regression was added during 3.19 development,
so it has not been in a released kernel, and so I did not cc
stable.
Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
drivers/scsi/device_handler/scsi_dh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 1dba62c..1efebc9 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -136,11 +136,12 @@ static void __detach_handler (struct kref *kref)
struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
struct scsi_device *sdev = scsi_dh_data->sdev;
+ scsi_dh->detach(sdev);
+
spin_lock_irq(sdev->request_queue->queue_lock);
sdev->scsi_dh_data = NULL;
spin_unlock_irq(sdev->request_queue->queue_lock);
- scsi_dh->detach(sdev);
sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", scsi_dh->name);
module_put(scsi_dh->module);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops
2015-01-28 9:46 [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops michaelc
@ 2015-01-29 8:45 ` Hannes Reinecke
2015-01-29 8:56 ` Christoph Hellwig
2015-01-29 8:59 ` Mike Christie
0 siblings, 2 replies; 4+ messages in thread
From: Hannes Reinecke @ 2015-01-29 8:45 UTC (permalink / raw)
To: michaelc, linux-scsi
On 01/28/2015 10:46 AM, michaelc@cs.wisc.edu wrote:
> From: Mike Christie <michaelc@cs.wisc.edu>
>
> This fixes a regression caused by commit
> 1d5203284d8acbdfdf9b478d434450b34f338f28
>
> The bug is that the alua detach() callout will try to access the
> sddev->scsi_dh_data, but we have already set it to NULL. This patch
> moves the clearing of that field to after detach() is called.
>
> It looks like the regression was added during 3.19 development,
> so it has not been in a released kernel, and so I did not cc
> stable.
>
> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
>
> ---
> drivers/scsi/device_handler/scsi_dh.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
> index 1dba62c..1efebc9 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
> @@ -136,11 +136,12 @@ static void __detach_handler (struct kref *kref)
> struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
> struct scsi_device *sdev = scsi_dh_data->sdev;
>
> + scsi_dh->detach(sdev);
> +
> spin_lock_irq(sdev->request_queue->queue_lock);
> sdev->scsi_dh_data = NULL;
> spin_unlock_irq(sdev->request_queue->queue_lock);
>
> - scsi_dh->detach(sdev);
> sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", scsi_dh->name);
> module_put(scsi_dh->module);
> }
>
Errm.
We save the contents first:
> struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
Then set the pointer to NULL:
> sdev->scsi_dh_data = NULL;
and then call 'detach':
> scsi_dh->detach(sdev);
So scsi_dh is _not_ NULL, hence it shouldn't oops.
NACK.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops
2015-01-29 8:45 ` Hannes Reinecke
@ 2015-01-29 8:56 ` Christoph Hellwig
2015-01-29 8:59 ` Mike Christie
1 sibling, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2015-01-29 8:56 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: michaelc, linux-scsi
On Thu, Jan 29, 2015 at 09:45:24AM +0100, Hannes Reinecke wrote:
> Errm.
>
> We save the contents first:
>
> > struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
>
> Then set the pointer to NULL:
>
> > sdev->scsi_dh_data = NULL;
>
> and then call 'detach':
>
> > scsi_dh->detach(sdev);
>
> So scsi_dh is _not_ NULL, hence it shouldn't oops.
The actual ->detach method can, and usually does access it.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops
2015-01-29 8:45 ` Hannes Reinecke
2015-01-29 8:56 ` Christoph Hellwig
@ 2015-01-29 8:59 ` Mike Christie
1 sibling, 0 replies; 4+ messages in thread
From: Mike Christie @ 2015-01-29 8:59 UTC (permalink / raw)
To: Hannes Reinecke, linux-scsi
On 01/29/2015 02:45 AM, Hannes Reinecke wrote:
> On 01/28/2015 10:46 AM, michaelc@cs.wisc.edu wrote:
>> From: Mike Christie <michaelc@cs.wisc.edu>
>>
>> This fixes a regression caused by commit
>> 1d5203284d8acbdfdf9b478d434450b34f338f28
>>
>> The bug is that the alua detach() callout will try to access the
>> sddev->scsi_dh_data, but we have already set it to NULL. This patch
>> moves the clearing of that field to after detach() is called.
>>
>> It looks like the regression was added during 3.19 development,
>> so it has not been in a released kernel, and so I did not cc
>> stable.
>>
>> Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
>>
>> ---
>> drivers/scsi/device_handler/scsi_dh.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
>> index 1dba62c..1efebc9 100644
>> --- a/drivers/scsi/device_handler/scsi_dh.c
>> +++ b/drivers/scsi/device_handler/scsi_dh.c
>> @@ -136,11 +136,12 @@ static void __detach_handler (struct kref *kref)
>> struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
>> struct scsi_device *sdev = scsi_dh_data->sdev;
>>
>> + scsi_dh->detach(sdev);
>> +
>> spin_lock_irq(sdev->request_queue->queue_lock);
>> sdev->scsi_dh_data = NULL;
>> spin_unlock_irq(sdev->request_queue->queue_lock);
>>
>> - scsi_dh->detach(sdev);
>> sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", scsi_dh->name);
>> module_put(scsi_dh->module);
>> }
>>
> Errm.
>
> We save the contents first:
>
>> struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
>
> Then set the pointer to NULL:
>
>> sdev->scsi_dh_data = NULL;
>
> and then call 'detach':
>
>> scsi_dh->detach(sdev);
>
> So scsi_dh is _not_ NULL, hence it shouldn't oops.
>
The problem is the actual detach() functions are the ones that are
accessing the NULL'd scsi_dh_data->scsi_dh pointer.
So above we have set sdev->scsi_dh_data to NULL and then are calling
detach(). In scsi_dh_alua.c, get_alua_data() we will then access the
NULL'd pointer.
static void alua_bus_detach(struct scsi_device *sdev)
{
struct alua_dh_data *h = get_alua_data(sdev);
if (h->buff && h->inq != h->buff)
kfree(h->buff);
kfree(h);
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-29 8:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-28 9:46 [PATCH 1/1] [PATCH REGRESSION] alua: fix bus detach oops michaelc
2015-01-29 8:45 ` Hannes Reinecke
2015-01-29 8:56 ` Christoph Hellwig
2015-01-29 8:59 ` Mike Christie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).