From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751557AbdCCB2U (ORCPT ); Thu, 2 Mar 2017 20:28:20 -0500 Received: from mail-lf0-f67.google.com ([209.85.215.67]:33099 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751150AbdCCB2S (ORCPT ); Thu, 2 Mar 2017 20:28:18 -0500 Subject: Re: [PATCH] virtio-console: avoid DMA from stack To: Omar Sandoval , Amit Shah References: <075115ddd99246ffc4228a3e050ba68eb36c6a8c.1485935920.git.osandov@fb.com> Cc: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, kernel-team@fb.com From: Jan Dakinevich Message-ID: <440dcce4-c3df-1ff5-1297-e901311c98a9@gmail.com> Date: Fri, 3 Mar 2017 03:58:53 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <075115ddd99246ffc4228a3e050ba68eb36c6a8c.1485935920.git.osandov@fb.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > 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 > --- > 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Dakinevich Subject: Re: [PATCH] virtio-console: avoid DMA from stack Date: Fri, 3 Mar 2017 03:58:53 +0300 Message-ID: <440dcce4-c3df-1ff5-1297-e901311c98a9@gmail.com> References: <075115ddd99246ffc4228a3e050ba68eb36c6a8c.1485935920.git.osandov@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <075115ddd99246ffc4228a3e050ba68eb36c6a8c.1485935920.git.osandov@fb.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Omar Sandoval , Amit Shah Cc: kernel-team@fb.com, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org List-Id: virtualization@lists.linuxfoundation.org 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 > > 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 > --- > 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