On Fri, 21 Sep 2018 14:46:21 +0200 Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > While ccw_io_helper() seems like intended to be exclusive in a sense that > it is supposed to facilitate I/O for at most one thread at any given > time, there is actually nothing ensuring that threads won't pile up at > vcdev->wait_q. If they all threads get woken up and see the status that s/If they/If they do,/ > belongs to some other request as their own. This can lead to bugs. For an s/as/than/ > example see : > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1788432 > > This normally does not cause problems, as these are usually infrequent > operations that happen in a well defined sequence and normally do not > fail. But occasionally sysfs attributes are directly dependent on pieces > of virio config and trigger a get on each read. This gives us at least > one method to trigger races. Again, I'd prefer a rewording, but a slightly different one: "These normally does not cause problems, as these are usually infrequent operations that happen in a well defined sequence and normally do not fail. But for example, sysfs attributes may trigger concurrent reading/writing of the config space, including read before write." (Not sure if that's better...) > > Let us fix the problem by ensuring, that for each device, we finish > processing the previous request before starting with a new one. > > Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx> > Reported-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> > --- > drivers/s390/virtio/virtio_ccw.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) Otherwise, looks good.
