| Message ID | 20230828013736.18414-2-jniethe5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] virtio-serial: Fix invalid stack access with closed virtio device | expand |
On 28/08/2023 03.37, Jordan Niethe wrote: > From: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > The read and write methods return as though they were successfully even I'd scratch "as though they were" from the sentence - that part makes it just harder to read IMHO. > if the virtio device is closed (virtiodev is 0) and it is not able to > send or receive any characters. > > Make the read and write methods return 0 to indicate they did not > succeed in this case. > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > board-qemu/slof/virtio-serial.fs | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs > index a67a310..41e2e04 100644 > --- a/board-qemu/slof/virtio-serial.fs > +++ b/board-qemu/slof/virtio-serial.fs > @@ -67,7 +67,7 @@ virtiodev virtio-serial-init drop > ; > > : write ( addr len -- actual ) > - virtiodev 0= IF nip EXIT THEN > + virtiodev 0= IF 2drop 0 EXIT THEN Ok, makes sense! > tuck > 0 ?DO > dup c@ virtiodev SWAP virtio-serial-putchar > @@ -78,7 +78,7 @@ virtiodev virtio-serial-init drop > > : read ( addr len -- actual ) > 0= IF drop 0 EXIT THEN > - virtiodev 0= IF drop 1 EXIT THEN > + virtiodev 0= IF drop 0 EXIT THEN Ah, ok, here's the 0 ... I'd suggest to use that in the first patch already. Thomas
On Mon, Aug 28, 2023 at 5:01 PM Thomas Huth <thuth@redhat.com> wrote: > > On 28/08/2023 03.37, Jordan Niethe wrote: > > From: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > > > The read and write methods return as though they were successfully even > > I'd scratch "as though they were" from the sentence - that part makes it > just harder to read IMHO. Sure. > > > if the virtio device is closed (virtiodev is 0) and it is not able to > > send or receive any characters. > > > > Make the read and write methods return 0 to indicate they did not > > succeed in this case. > > > > Signed-off-by: Kautuk Consul <kconsul@linux.vnet.ibm.com> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > > --- > > board-qemu/slof/virtio-serial.fs | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs > > index a67a310..41e2e04 100644 > > --- a/board-qemu/slof/virtio-serial.fs > > +++ b/board-qemu/slof/virtio-serial.fs > > @@ -67,7 +67,7 @@ virtiodev virtio-serial-init drop > > ; > > > > : write ( addr len -- actual ) > > - virtiodev 0= IF nip EXIT THEN > > + virtiodev 0= IF 2drop 0 EXIT THEN > > Ok, makes sense! > > > tuck > > 0 ?DO > > dup c@ virtiodev SWAP virtio-serial-putchar > > @@ -78,7 +78,7 @@ virtiodev virtio-serial-init drop > > > > : read ( addr len -- actual ) > > 0= IF drop 0 EXIT THEN > > - virtiodev 0= IF drop 1 EXIT THEN > > + virtiodev 0= IF drop 0 EXIT THEN > > Ah, ok, here's the 0 ... I'd suggest to use that in the first patch already. Just wanted to seperate fixing the incorrectly implemented desired behaviour of commit 8174acd ("virtio-serial: Close device completely") in which the virtiodev == 0 case returns successfully and changing that to return unsuccessfully, to check if there was some other situation Alexey had had in mind for that behaviour. First patch can be dropped. > > Thomas > > _______________________________________________ > SLOF mailing list > SLOF@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/slof
diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs index a67a310..41e2e04 100644 --- a/board-qemu/slof/virtio-serial.fs +++ b/board-qemu/slof/virtio-serial.fs @@ -67,7 +67,7 @@ virtiodev virtio-serial-init drop ; : write ( addr len -- actual ) - virtiodev 0= IF nip EXIT THEN + virtiodev 0= IF 2drop 0 EXIT THEN tuck 0 ?DO dup c@ virtiodev SWAP virtio-serial-putchar @@ -78,7 +78,7 @@ virtiodev virtio-serial-init drop : read ( addr len -- actual ) 0= IF drop 0 EXIT THEN - virtiodev 0= IF drop 1 EXIT THEN + virtiodev 0= IF drop 0 EXIT THEN virtiodev virtio-serial-haschar 0= IF 0 swap c! -2 EXIT THEN virtiodev virtio-serial-getchar swap c! 1 ;