linux-kernel - Re: [PATCH v2 2/3] usb: gadget: file_storage: Make CD-ROM emulation work with Mac OS-X

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] [day] [month] [year] [list]

Message-ID: <op.vss0p8h93l0zgt@mnazarewicz-glaptop>
Date:	Wed, 23 Mar 2011 17:14:22 +0100
From:	"Michal Nazarewicz" <mina86@...a86.com>
To:	"Roger Quadros" <roger.quadros@...ia.com>,
	"Alan Stern" <stern@...land.harvard.edu>
Cc:	gregkh@...e.de, sshtylyov@...sta.com,
	"USB list" <linux-usb@...r.kernel.org>,
	"Kernel development list" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/3] usb: gadget: file_storage: Make CD-ROM emulation
 work with Mac OS-X

>>> On the other hand, I don't think any implementations would get upset  
>>> if we simply ended the transfer with a short packet instead of
>>> adhering strictly to the spec.

> On Wed, 23 Mar 2011, Roger Quadros wrote:
>> I tried your patch with the CD-ROM implementation and it works  
>> perfectly. I do not see the unnecessary zero padded transfers
>> any more.
>>
>> Do you think we should have this patch in? with the risk of not strictly
>> adhering to spec for cases where controller cannot stall?

On Wed, 23 Mar 2011 16:17:32 +0100, Alan Stern wrote:
> There already is another place where not stalling forces the driver to
> violate the spec.  I don't think this makes things much worse... but it
> is a significant change in behavior.
>
> This affects Michal's driver too; we should ask his opinion.  Michal,
> in case you didn't see it, the proposed patch is here:
>
> 	http://marc.info/?l=linux-usb&m=130080683528607&w=2

Thanks.  I don't have original message so I'll reply here:

> +		/* Don't know what to do if common->fsg is NULL */
> +		} else if (!common->fsg) {
> +			rc = -EIO;
> +

I'd change that to "if (!fsg_is_set(common))".  fsg_is_set() prints out a  
warning
if common->fsg is not set and I think that's something we want.

(Come to think of it, this check should be done at the beginning of the  
function
simplifying the rest of the function, but that's unrelated, I'll do it  
myself I
guess.)

Other then that, looks good to me.

As for violating the spec, I don't think I have anything clever to add...

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michal "mina86" Nazarewicz    (o o)
ooo +-----<email/xmpp: mnazarewicz@...gle.com>-----ooO--(_)--Ooo--
--
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/