On Mon, Feb 27, 2023 at 05:58:12PM -0800, Linus Torvalds wrote: > On Mon, Feb 27, 2023 at 5:46 PM <linux@xxxxxxxxxxxxxx> wrote: > > > > This still seems to be broken for me. > > Looks that way. > > > I still need the patch from > > > > https://lore.kernel.org/lkml/20230220064612.1783-1-linux@xxxxxxxxxxxxxx/ > > .. and that has the same problem with "what if the error happens > during an iteration that wasn't the first, and we already succeeded > partially". Indeed. > The "goto unlock_out" is bogus, since it jumps over all the "update > pos and check if we read something". > > It was the correct thing to do *above* the loop, but not inside the loop. > > IOW, I think the proper patch is to also turn the "goto unlock_out" > into a "break". Mind testing something like this (whitespace-damaged, > but you get the idea): Makes sense and seems to work correctly. Tested-By: Thomas Weißschuh <linux@xxxxxxxxxxxxxx> (Or feel free to use my patch from above and fixup the goto/break line) > > --- a/drivers/tty/vt/vc_screen.c > +++ b/drivers/tty/vt/vc_screen.c > @@ -403,10 +403,11 @@ vcs_read(struct file *file, char __user > *buf, size_t count, loff_t *ppos) > unsigned int this_round, skip = 0; > int size; > > - ret = -ENXIO; > vc = vcs_vc(inode, &viewed); > - if (!vc) > - goto unlock_out; > + if (!vc) { > + ret = -ENXIO; > + break; > + } > > /* Check whether we are above size each round, > * as copy_to_user at the end of this loop > > which hopefully really fixes this (at least I don't see any other > "goto unlock_out" cases). > > Linus
