From: Omar Sandoval <osandov at fb.com>
put_chars() stuffs the buffer it gets into an sg, but that buffer may be
on the stack. This breaks with CONFIG_VMAP_STACK=y (for me, it
manifested as printks getting turned into NUL bytes).
Signed-off-by: Omar Sandoval <osandov at fb.com>
---
Patch based on v4.10-rc6.
drivers/char/virtio_console.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 17857beb4892..3cbf4c95e446 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1136,6 +1136,8 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
{
struct port *port;
struct scatterlist sg[1];
+ void *data;
+ int ret;
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
@@ -1144,8 +1146,14 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
if (!port)
return -EPIPE;
- sg_init_one(sg, buf, count);
- return __send_to_port(port, sg, 1, count, (void *)buf, false);
+ data = kmemdup(buf, count, GFP_ATOMIC);
+ if (!data)
+ return -ENOMEM;
+
+ sg_init_one(sg, data, count);
+ ret = __send_to_port(port, sg, 1, count, data, false);
+ kfree(data);
+ return ret;
}
/*
--
2.11.0
On (Wed) 01 Feb 2017 [00:02:27], Omar Sandoval wrote:> From: Omar Sandoval <osandov at fb.com> > > put_chars() stuffs the buffer it gets into an sg, but that buffer may be > on the stack. This breaks with CONFIG_VMAP_STACK=y (for me, it > manifested as printks getting turned into NUL bytes).Seems reasonable. I wonder since all implementations of hvc do a memcpy, if we can abstract it - but that'll need some work. Reviewed-by: Amit Shah <amit.shah at redhat.com> Michael, please add to the virtio queue. Amit
On Wed, Feb 01, 2017 at 07:17:12PM +0530, Amit Shah wrote:> On (Wed) 01 Feb 2017 [00:02:27], Omar Sandoval wrote: > > From: Omar Sandoval <osandov at fb.com> > > > > put_chars() stuffs the buffer it gets into an sg, but that buffer may be > > on the stack. This breaks with CONFIG_VMAP_STACK=y (for me, it > > manifested as printks getting turned into NUL bytes). > > Seems reasonable. I wonder since all implementations of hvc do a > memcpy, if we can abstract it - but that'll need some work. > > Reviewed-by: Amit Shah <amit.shah at redhat.com> > > Michael, please add to the virtio queue. > > AmitHi, Michael, I don't see this in Linus' master or in your git tree, is it going to go in for -rc1? Thanks.
I also faced with the same issue. Could you clarify it for me whether it is safe to allocate memory inside console driver handler? For example, what would happen if put_chars was triggered by fail in another memory allocation? On 02/01/2017 11:02 AM, Omar Sandoval wrote:> From: Omar Sandoval <osandov at fb.com> > > put_chars() stuffs the buffer it gets into an sg, but that buffer may be > on the stack. This breaks with CONFIG_VMAP_STACK=y (for me, it > manifested as printks getting turned into NUL bytes). > > Signed-off-by: Omar Sandoval <osandov at fb.com> > --- > Patch based on v4.10-rc6. > > drivers/char/virtio_console.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c > index 17857beb4892..3cbf4c95e446 100644 > --- a/drivers/char/virtio_console.c > +++ b/drivers/char/virtio_console.c > @@ -1136,6 +1136,8 @@ static int put_chars(u32 vtermno, const char *buf, int count) > { > struct port *port; > struct scatterlist sg[1]; > + void *data; > + int ret; > > if (unlikely(early_put_chars)) > return early_put_chars(vtermno, buf, count); > @@ -1144,8 +1146,14 @@ static int put_chars(u32 vtermno, const char *buf, int count) > if (!port) > return -EPIPE; > > - sg_init_one(sg, buf, count); > - return __send_to_port(port, sg, 1, count, (void *)buf, false); > + data = kmemdup(buf, count, GFP_ATOMIC); > + if (!data) > + return -ENOMEM; > + > + sg_init_one(sg, data, count); > + ret = __send_to_port(port, sg, 1, count, data, false); > + kfree(data); > + return ret; > } > > /* >-- Best regards, Jan Dakinevich