Re: [RFC 0/5] fix races in CDC-WDM

[Oliver Neukum <oneukum@suse.com>]

Am Dienstag, den 22.09.2020, 10:56 +0900 schrieb Tetsuo Handa:
> On 2020/09/21 19:52, Oliver Neukum wrote:

> 
> To understand it, I must understand why it is safe to defer error reporting.

It is not. There is nothing to understand here. If user space needs
a guarantee that data has been pushed out without an error, it will
have to call fsync()

Let me explain.
POSIX, as described in the man page, sets some rules about what
a driver must do and must not do. In other areas it makes
recommendations a good driver should follow. One of them is that
we block as little as possible, at the risk of delaying error
reporting. A good driver will report errors as soon as possible
while being compatible with doing IO asynchronously.

> I'm querying you about characteristics of data passed to wdm_write().
> Without knowing the difference between writing to cdc-wdm driver and normal file on
> some filesystem, I can't judge whether it is acceptable to defer reporting errors.

That is simply not a decision you or I make. The man page clearly
says that it is acceptable. If user space does not like that, it must
call fsync() after write().

> When an error occurred when writing to normal file on some filesystem, the userspace
> would simply treat that file as broken (e.g. delete such file). The userspace of this
> cdc-wdm driver might want that any error is immediately reported, for I'm thinking that
> each data passed to wdm_write() is stateful, for
> 
>   /* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */
>   #define WDM_DEFAULT_BUFSIZE     256
> 
> and
> 
>         if (count > desc->wMaxCommand)
>                 count = desc->wMaxCommand;
> 
> makes me think that wdm_write() has to be able to handle chunk of data in atomic
> manner.

No. the WDM spec says the clear opposite and the man page of write says
so, too.

> I don't like [RFC 8/8]. Please drop [RFC 8/8] because not only it is unrelated to
> hang up problem syzbot is reporting but also lock dependency is still unclear.

Very well. I will make a patch set that clearly fixes bugs and a
secondary one with optimizations.

	Regards
		Oliver