linux-kernel - Re: [PATCH] usb: gadget: storage: optional SCSI WRITE FUA bit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.44L0.1007131003560.1568-100000@iolanthe.rowland.org>
Date:	Tue, 13 Jul 2010 10:09:19 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
cc:	linux-kernel@...r.kernel.org,
	Andy Shevchenko <ext-andriy.shevchenko@...ia.com>,
	Denis Karpov <ext-denis.2.karpov@...ia.com>,
	Adrian Hunter <adrian.hunter@...ia.com>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	<linux-usb@...r.kernel.org>
Subject: Re: [PATCH] usb: gadget: storage: optional SCSI WRITE FUA bit

On Tue, 13 Jul 2010, Andy Shevchenko wrote:

> MS Windows mounts removable storage in "Removal optimized mode" by
> default. All the writes to the media are synchronous which is achieved
> by setting FUA (Force Unit Access) bit in SCSI WRITE(10,12) commands.
> This prevents I/O requests aggregation in block layer dramatically
> decreasing performance.
> 
> This patch brings an option to accept or ignore mentioned bit
>  a) via specifying module parameter "fua", or
>  b) through sysfs entry
> 	/sys/devices/platform/musb_hdrc/gadget/gadget-lun-N/fua

...

> @@ -111,11 +113,11 @@
>   *					PAGE_CACHE_SIZE)
>   *
>   * If CONFIG_USB_FILE_STORAGE_TEST is not set, only the "file", "ro",
> - * "removable", "luns", "stall", and "cdrom" options are available; default
> + * "removable", "luns", "fua", "stall", and "cdrom" options are available; default
>   * values are used for everything else.
>   *
>   * The pathnames of the backing files and the ro settings are available in
> - * the attribute files "file" and "ro" in the lun<n> subdirectory of the
> + * the attribute files "file", "fua", and "ro" in the lun<n> subdirectory of the
>   * gadget's sysfs directory.  If the "removable" option is set, writing to
>   * these files will simulate ejecting/loading the medium (writing an empty
>   * line means eject) and adjusting a write-enable tab.  Changes to the ro

Please do not create lines longer than 80 columns.  Reflow the text.

> @@ -307,6 +309,7 @@ static struct {
>  
>  	int		removable;
>  	int		can_stall;
> +	int		fua;
>  	int		cdrom;
>  
>  	char		*transport_parm;
> @@ -326,6 +329,7 @@ static struct {
>  	.protocol_parm		= "SCSI",
>  	.removable		= 0,
>  	.can_stall		= 1,
> +	.fua			= 1,
>  	.cdrom			= 0,
>  	.vendor			= FSG_VENDOR_ID,
>  	.product		= FSG_PRODUCT_ID,
> @@ -350,6 +354,9 @@ MODULE_PARM_DESC(removable, "true to simulate removable media");
>  module_param_named(stall, mod_data.can_stall, bool, S_IRUGO);
>  MODULE_PARM_DESC(stall, "false to prevent bulk stalls");
>  
> +module_param_named(fua, mod_data.fua, bool, S_IRUGO);
> +MODULE_PARM_DESC(fua, "true to obey SCSI WRITE(6,10,12) FUA bit");
> +
>  module_param_named(cdrom, mod_data.cdrom, bool, S_IRUGO);
>  MODULE_PARM_DESC(cdrom, "true to emulate cdrom instead of disk");

This implementation is wrong.  If "fua" is supposed to be per-lun 
then the module parameter needs to accept an array of values, like the 
"file" and "ro" parameters.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/