Re: [PATCH] scsi: introduce a quirk for false cache reporting

2016-08-02 Thread Johannes Thumshirn 
On Tue, Jul 12, 2016 at 10:24:36AM +0200, Oliver Neukum wrote:
> Some SATA to USB bridges fail to cooperate with some
> drives resulting in no cache being present being reported
> to the host. That causes the host to skip sending
> a command to synchronize caches. That causes data loss
> when the drive is powered down.
> 
> Signed-off-by: Oliver Neukum 

Reviewed-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [-next] BUG_ON in scsi_target_destroy()

2016-04-13 Thread Johannes Thumshirn 
Hi Sergey,  Xiong,

Can you try below patch?

On Montag, 11. April 2016 18:01:47 CEST Sergey Senozhatsky wrote:
> Hello,
> 
> commit 7b106f2de6938c31ce5e9c86bc70ad3904666b96
> Author: Johannes Thumshirn 
> Date:   Tue Apr 5 11:50:44 2016 +0200
> 
> scsi: Add intermediate STARGET_REMOVE state to scsi_target_state
> 
> 
> BUG_ON()s (next-20160411) each time I remove a usb flash
> 
> [   49.561600]  [] scsi_target_destroy+0x5a/0xcb [scsi_mod]
> [   49.561607]  [] scsi_target_reap+0x4a/0x4f [scsi_mod]
> [   49.561613]  [] __scsi_remove_device+0xc3/0xd0 [scsi_mod]
> [   49.561619]  [] scsi_forget_host+0x52/0x63 [scsi_mod]
> [   49.561623]  [] scsi_remove_host+0x8c/0x102 [scsi_mod]
> [   49.561627]  [] usb_stor_disconnect+0x6b/0xab 
> [usb_storage]
> [   49.561634]  [] usb_unbind_interface+0x77/0x1ca [usbcore]
> [   49.561636]  [] __device_release_driver+0x9d/0x121
> [   49.561638]  [] device_release_driver+0x23/0x30
> [   49.561639]  [] bus_remove_device+0xfb/0x10e
> [   49.561641]  [] device_del+0x164/0x1e6
> [   49.561648]  [] ? remove_intf_ep_devs+0x3b/0x48 [usbcore]
> [   49.561655]  [] usb_disable_device+0x84/0x1a5 [usbcore]
> [   49.561661]  [] usb_disconnect+0x94/0x19f [usbcore]
> [   49.561667]  [] hub_event+0x5c1/0xdea [usbcore]
> [   49.561670]  [] process_one_work+0x1dc/0x37f
> [   49.561672]  [] worker_thread+0x282/0x36d
> [   49.561673]  [] ? rescuer_thread+0x2ae/0x2ae
> [   49.561675]  [] kthread+0xd2/0xda
> [   49.561678]  [] ret_from_fork+0x22/0x40
> [   49.561679]  [] ? kthread_worker_fn+0x13e/0x13e
> 
>   -ss
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 0734927..0c00928 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1276,6 +1276,7 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 void __scsi_remove_device(struct scsi_device *sdev)
 {
struct device *dev = &sdev->sdev_gendev;
+   struct scsi_target *starget;
 
/*
 * This cleanup path is not reentrant and while it is impossible
@@ -1315,7 +1316,9 @@ void __scsi_remove_device(struct scsi_device *sdev)
 * remoed sysfs visibility from the device, so make the target
 * invisible if this was the last device underneath it.
 */
-   scsi_target_reap(scsi_target(sdev));
+   starget = scsi_target(sdev);
+   starget->state = STARGET_REMOVE;
+   scsi_target_reap(starget);
 
put_device(dev);
 }



-- 
Johannes Thumshirn  
  Storage
jthumsh...@suse.de 
+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [-next] BUG_ON in scsi_target_destroy()

2016-04-13 Thread Johannes Thumshirn 
On Mittwoch, 13. April 2016 23:47:04 CEST Sergey Senozhatsky wrote:
> Hi,
> 
> On (04/13/16 10:41), Johannes Thumshirn wrote:
> > Hi Sergey,  Xiong,
> > 
> > Can you try below patch?
> 
> it panics my system.
> first it warn_on-s in lib/kobject.c:244
> 
> then NULL dereferences
> do_scan_async
>  __scsi_remove_device
>   scsi_target_reap
>device_del
> dpm_sysfs_remove
>  sysfs_unmerge_group
>   kernfs_find_and_get_ns
>kernfs_find_ns
> 
>   -ss
> 

Thanks for the update, I'll have a look into the callchain

-- 
Johannes Thumshirn  
  Storage
jthumsh...@suse.de 
+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t

2017-03-06 Thread Johannes Thumshirn 
On 03/06/2017 03:21 PM, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.

The subject is wrong, should be something like "scsi: libfc convert
fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390.

Other than that
Acked-by: Johannes Thumshirn 

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t

2017-03-08 Thread Johannes Thumshirn 

On 03/08/2017 02:48 PM, Reshetova, Elena wrote:

On 03/06/2017 03:21 PM, Elena Reshetova wrote:

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.


The subject is wrong, should be something like "scsi: libfc convert
fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390.

Other than that
Acked-by: Johannes Thumshirn 


Turns out that it is better that all these patches go through the respective 
maintainer trees, if present.
If I send an updated patch (with subject fixed), could you merge it through 
your tree?


Yes, but this would be the normal scsi tree from Martin and James.

Please include my Ack in the re-sends.

Thanks a lot,
Johannes


--
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

2017-03-09 Thread Johannes Thumshirn 
On 03/09/2017 08:18 AM, Reshetova, Elena wrote:
>> On Mon, Mar 06, 2017 at 04:21:09PM +0200, Elena Reshetova wrote:
>>> refcount_t type and corresponding API should be
>>> used instead of atomic_t when the variable is used as
>>> a reference counter. This allows to avoid accidental
>>> refcounter overflows that might lead to use-after-free
>>> situations.
>>>
>>> Signed-off-by: Elena Reshetova 
>>> Signed-off-by: Hans Liljestrand 
>>> Signed-off-by: Kees Cook 
>>> Signed-off-by: David Windsor 
>>
>> This looks OK to me.
>>
>> Acked-by: Chris Leech 
> 
> Thank you for review! Do you have a tree that can take this change? 

Hi Elena,

iscsi like fcoe should go via the SCSI tree.

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

2017-03-09 Thread Johannes Thumshirn 
On 03/09/2017 10:26 AM, Reshetova, Elena wrote:
> 
>> On 03/09/2017 08:18 AM, Reshetova, Elena wrote:
>>>> On Mon, Mar 06, 2017 at 04:21:09PM +0200, Elena Reshetova wrote:
>>>>> refcount_t type and corresponding API should be
>>>>> used instead of atomic_t when the variable is used as
>>>>> a reference counter. This allows to avoid accidental
>>>>> refcounter overflows that might lead to use-after-free
>>>>> situations.
>>>>>
>>>>> Signed-off-by: Elena Reshetova 
>>>>> Signed-off-by: Hans Liljestrand 
>>>>> Signed-off-by: Kees Cook 
>>>>> Signed-off-by: David Windsor 
>>>>
>>>> This looks OK to me.
>>>>
>>>> Acked-by: Chris Leech 
>>>
>>> Thank you for review! Do you have a tree that can take this change?
>>
>> Hi Elena,
>>
>> iscsi like fcoe should go via the SCSI tree.
> 
> Thanks Johannes! Should I resend with "Acked-by" added in order for it to be 
> picked up? 

Yes I think this would be a good way to go.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] usb: hub: avoid possible division by zero

2013-11-23 Thread Johannes Thumshirn 
Avoid possible division by zero in led_work, if hdev->maxchild is 0.

See also:
http://buildbot.llvm.linuxfoundation.org/checker/scan-build-latest/report-b65939.html#EndPath

Signed-off-by: Johannes Thumshirn 
---
 drivers/usb/core/hub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a7c04e2..8c7aa4e 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -453,7 +453,8 @@ static void led_work (struct work_struct *work)
unsignedchanged = 0;
int cursor = -1;
 
-   if (hdev->state != USB_STATE_CONFIGURED || hub->quiescing)
+   if (hdev->state != USB_STATE_CONFIGURED || hub->quiescing
+   || hdev->maxchild == 0)
return;
 
for (i = 0; i < hdev->maxchild; i++) {
-- 
1.8.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: hub: avoid possible division by zero

2013-11-23 Thread Johannes Thumshirn 
On Sat, Nov 23, 2013 at 09:20:10AM -0800, Greg Kroah-Hartman wrote:
> On Sat, Nov 23, 2013 at 05:31:34PM +0100, Johannes Thumshirn wrote:
> > Avoid possible division by zero in led_work, if hdev->maxchild is 0.
> >
> > See also:
> > http://buildbot.llvm.linuxfoundation.org/checker/scan-build-latest/report-b65939.html#EndPath
> >
> > Signed-off-by: Johannes Thumshirn 
> > ---
> >  drivers/usb/core/hub.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index a7c04e2..8c7aa4e 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -453,7 +453,8 @@ static void led_work (struct work_struct *work)
> > unsignedchanged = 0;
> > int cursor = -1;
> >
> > -   if (hdev->state != USB_STATE_CONFIGURED || hub->quiescing)
> > +   if (hdev->state != USB_STATE_CONFIGURED || hub->quiescing
> > +   || hdev->maxchild == 0)
>
> And how can maxchild ever be equal to 0?
>
> Look at where it is assigned (it comes from bNbrPorts in the
> descriptor), in hub_configure() and if it is set to 0, the creation of
> the structure fails and then this code will never run, right?
>
> So I don't think this is needed, and your checker needs to be fixed.
>
> sorry,

Args, sorry for the noise. Should've checked that better.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] cdc-acm: Destroy acm_minors IDR on module exit

2015-07-08 Thread Johannes Thumshirn 
Destroy acm_minors IDR on module exit, reclaiming the allocated memory.

This was detected by the following semantic patch (written by Luis Rodriguez
)

@ defines_module_init @
declarer name module_init, module_exit;
declarer name DEFINE_IDR;
identifier init;
@@

module_init(init);

@ defines_module_exit @
identifier exit;
@@

module_exit(exit);

@ declares_idr depends on defines_module_init && defines_module_exit @
identifier idr;
@@

DEFINE_IDR(idr);

@ on_exit_calls_destroy depends on declares_idr && defines_module_exit @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 idr_destroy(&idr);
 ...
}

@ missing_module_idr_destroy depends on declares_idr && defines_module_exit && 
!on_exit_calls_destroy @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 +idr_destroy(&idr);
 }


Signed-off-by: Johannes Thumshirn 
---
 drivers/usb/class/cdc-acm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 519a77b..b30e742 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1944,6 +1944,7 @@ static void __exit acm_exit(void)
usb_deregister(&acm_driver);
tty_unregister_driver(acm_tty_driver);
put_tty_driver(acm_tty_driver);
+   idr_destroy(&acm_minors);
 }
 
 module_init(acm_init);
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] USB: serial: Destroy serial_minors IDR on module exit

2015-07-08 Thread Johannes Thumshirn 
Destroy serial_minors IDR on module exit, reclaiming the allocated memory.

This was detected by the following semantic patch (written by Luis Rodriguez
)

@ defines_module_init @
declarer name module_init, module_exit;
declarer name DEFINE_IDR;
identifier init;
@@

module_init(init);

@ defines_module_exit @
identifier exit;
@@

module_exit(exit);

@ declares_idr depends on defines_module_init && defines_module_exit @
identifier idr;
@@

DEFINE_IDR(idr);

@ on_exit_calls_destroy depends on declares_idr && defines_module_exit @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 idr_destroy(&idr);
 ...
}

@ missing_module_idr_destroy depends on declares_idr && defines_module_exit && 
!on_exit_calls_destroy @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 +idr_destroy(&idr);
}


Signed-off-by: Johannes Thumshirn 
---
 drivers/usb/serial/usb-serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 529066b..46f1f13 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1306,6 +1306,7 @@ static void __exit usb_serial_exit(void)
tty_unregister_driver(usb_serial_tty_driver);
put_tty_driver(usb_serial_tty_driver);
bus_unregister(&usb_serial_bus_type);
+   idr_destroy(&serial_minors);
 }
 
 
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: broken TRIM support for JMS578 in uas mode

2018-08-02 Thread Johannes Thumshirn 
On Thu, Aug 02, 2018 at 02:24:16PM +0200, Oliver Neukum wrote:
> On Mo, 2018-07-30 at 14:43 +0300, Mailing Lists wrote:
> > I cannot issue TRIM commands to SSD behind a JMS578-based sata to
> > usb-c adapter. Tried with Fedora 28 kernel and with latest
> > 4.18.0-0.rc6.git0.1.vanilla.knurd.1.fc28.x86_64
> > lsblk -D shows that discard is not enabled, but the SSD has this
> > capability (see below)
> 
> Hi Johannes,
> 
> is there a way to signal the SCSI layer that a trim for a device should
> be attempted, even if it claims not to support it?

Generally speaking no. If the USB Adapter doesn't translate the
Command it doesn't translate it.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html