* [PATCH] virtio-console: avoid DMA from stack
@ 2017-02-01 8:02 Omar Sandoval
2017-02-01 13:47 ` Amit Shah
2017-03-03 0:58 ` Jan Dakinevich
0 siblings, 2 replies; 5+ messages in thread
From: Omar Sandoval @ 2017-02-01 8:02 UTC (permalink / raw)
To: Amit Shah; +Cc: virtualization, linux-kernel, kernel-team
From: Omar Sandoval <osandov@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@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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-console: avoid DMA from stack
2017-02-01 8:02 [PATCH] virtio-console: avoid DMA from stack Omar Sandoval
@ 2017-02-01 13:47 ` Amit Shah
2017-03-01 20:01 ` Omar Sandoval
2017-03-03 0:58 ` Jan Dakinevich
1 sibling, 1 reply; 5+ messages in thread
From: Amit Shah @ 2017-02-01 13:47 UTC (permalink / raw)
To: Omar Sandoval
Cc: virtualization, linux-kernel, kernel-team, Michael S. Tsirkin
On (Wed) 01 Feb 2017 [00:02:27], Omar Sandoval wrote:
> From: Omar Sandoval <osandov@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@redhat.com>
Michael, please add to the virtio queue.
Amit
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-console: avoid DMA from stack
2017-02-01 13:47 ` Amit Shah
@ 2017-03-01 20:01 ` Omar Sandoval
2017-03-01 21:25 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Omar Sandoval @ 2017-03-01 20:01 UTC (permalink / raw)
To: Michael S. Tsirkin, Amit Shah; +Cc: virtualization, linux-kernel, kernel-team
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@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@redhat.com>
>
> Michael, please add to the virtio queue.
>
> Amit
Hi, Michael,
I don't see this in Linus' master or in your git tree, is it going to go
in for -rc1?
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-console: avoid DMA from stack
2017-03-01 20:01 ` Omar Sandoval
@ 2017-03-01 21:25 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2017-03-01 21:25 UTC (permalink / raw)
To: Omar Sandoval; +Cc: Amit Shah, virtualization, linux-kernel, kernel-team
On Wed, Mar 01, 2017 at 12:01:10PM -0800, Omar Sandoval wrote:
> 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@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@redhat.com>
> >
> > Michael, please add to the virtio queue.
> >
> > Amit
>
> Hi, Michael,
>
> I don't see this in Linus' master or in your git tree, is it going to go
> in for -rc1?
>
> Thanks.
Yes.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio-console: avoid DMA from stack
2017-02-01 8:02 [PATCH] virtio-console: avoid DMA from stack Omar Sandoval
2017-02-01 13:47 ` Amit Shah
@ 2017-03-03 0:58 ` Jan Dakinevich
1 sibling, 0 replies; 5+ messages in thread
From: Jan Dakinevich @ 2017-03-03 0:58 UTC (permalink / raw)
To: Omar Sandoval, Amit Shah; +Cc: virtualization, linux-kernel, kernel-team
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@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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-03 1:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-01 8:02 [PATCH] virtio-console: avoid DMA from stack Omar Sandoval
2017-02-01 13:47 ` Amit Shah
2017-03-01 20:01 ` Omar Sandoval
2017-03-01 21:25 ` Michael S. Tsirkin
2017-03-03 0:58 ` Jan Dakinevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).