* [RFC] sandbox: prevent segfault in tap_alloc() @ 2019-06-19 10:49 Antony Pavlov 2019-06-26 7:12 ` Sascha Hauer 0 siblings, 1 reply; 4+ messages in thread From: Antony Pavlov @ 2019-06-19 10:49 UTC (permalink / raw) To: barebox; +Cc: Oleksij Rempel Tap network interface initialization in sandbox barebox leads to segfault under Debian Buster/Sid. The problem is that strcpy(dev, ifr.ifr_name) inside tap_alloc() tries to alter read-only data passed by tap_probe() and barebox receives SIGSEGV. Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> --- drivers/net/tap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index 1fbfa085b1..d7e32f4875 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -14,7 +14,7 @@ struct tap_priv { int fd; - char *name; + char name[128]; }; static int tap_eth_send(struct eth_device *edev, void *packet, int length) @@ -65,7 +65,7 @@ static int tap_probe(struct device_d *dev) int ret = 0; priv = xzalloc(sizeof(struct tap_priv)); - priv->name = "barebox"; + strncpy(priv->name, "barebox", sizeof(priv->name)); priv->fd = tap_alloc(priv->name); if (priv->fd < 0) { -- 2.20.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] sandbox: prevent segfault in tap_alloc() 2019-06-19 10:49 [RFC] sandbox: prevent segfault in tap_alloc() Antony Pavlov @ 2019-06-26 7:12 ` Sascha Hauer 2019-06-28 8:32 ` Antony Pavlov 0 siblings, 1 reply; 4+ messages in thread From: Sascha Hauer @ 2019-06-26 7:12 UTC (permalink / raw) To: Antony Pavlov; +Cc: barebox, Oleksij Rempel Hi Antony, On Wed, Jun 19, 2019 at 01:49:16PM +0300, Antony Pavlov wrote: > Tap network interface initialization in sandbox > barebox leads to segfault under Debian Buster/Sid. > > The problem is that strcpy(dev, ifr.ifr_name) inside > tap_alloc() tries to alter read-only data passed > by tap_probe() and barebox receives SIGSEGV. > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > --- > drivers/net/tap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > index 1fbfa085b1..d7e32f4875 100644 > --- a/drivers/net/tap.c > +++ b/drivers/net/tap.c > @@ -14,7 +14,7 @@ > > struct tap_priv { > int fd; > - char *name; > + char name[128]; > }; > > static int tap_eth_send(struct eth_device *edev, void *packet, int length) > @@ -65,7 +65,7 @@ static int tap_probe(struct device_d *dev) > int ret = 0; > > priv = xzalloc(sizeof(struct tap_priv)); > - priv->name = "barebox"; > + strncpy(priv->name, "barebox", sizeof(priv->name)); > > priv->fd = tap_alloc(priv->name); Can we change the prototype of tap_alloc() to something like this: int tap_alloc(const char *name, int *fd, char **outname); outname would be an allocated string to be freed by the caller. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] sandbox: prevent segfault in tap_alloc() 2019-06-26 7:12 ` Sascha Hauer @ 2019-06-28 8:32 ` Antony Pavlov 2019-06-28 9:04 ` Sascha Hauer 0 siblings, 1 reply; 4+ messages in thread From: Antony Pavlov @ 2019-06-28 8:32 UTC (permalink / raw) To: Sascha Hauer; +Cc: barebox, Oleksij Rempel On Wed, 26 Jun 2019 09:12:02 +0200 Sascha Hauer <s.hauer@pengutronix.de> wrote: > Hi Antony, > > On Wed, Jun 19, 2019 at 01:49:16PM +0300, Antony Pavlov wrote: > > Tap network interface initialization in sandbox > > barebox leads to segfault under Debian Buster/Sid. > > > > The problem is that strcpy(dev, ifr.ifr_name) inside > > tap_alloc() tries to alter read-only data passed > > by tap_probe() and barebox receives SIGSEGV. > > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > > --- > > drivers/net/tap.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > > index 1fbfa085b1..d7e32f4875 100644 > > --- a/drivers/net/tap.c > > +++ b/drivers/net/tap.c > > @@ -14,7 +14,7 @@ > > > > struct tap_priv { > > int fd; > > - char *name; > > + char name[128]; > > }; > > > > static int tap_eth_send(struct eth_device *edev, void *packet, int length) > > @@ -65,7 +65,7 @@ static int tap_probe(struct device_d *dev) > > int ret = 0; > > > > priv = xzalloc(sizeof(struct tap_priv)); > > - priv->name = "barebox"; > > + strncpy(priv->name, "barebox", sizeof(priv->name)); > > > > priv->fd = tap_alloc(priv->name); > > Can we change the prototype of tap_alloc() to something like this: > > int tap_alloc(const char *name, int *fd, char **outname); > > outname would be an allocated string to be freed by the caller. There is one problem. tap_alloc works in the sandbox "os domain" (glibc *alloc&free etc), the caller works in the "barebox domain" (barebox *alloc&free). Can we just drop this outname? nobody actually use it at the moment. -- Best regards, Antony Pavlov _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] sandbox: prevent segfault in tap_alloc() 2019-06-28 8:32 ` Antony Pavlov @ 2019-06-28 9:04 ` Sascha Hauer 0 siblings, 0 replies; 4+ messages in thread From: Sascha Hauer @ 2019-06-28 9:04 UTC (permalink / raw) To: Antony Pavlov; +Cc: barebox, Oleksij Rempel On Fri, Jun 28, 2019 at 11:32:26AM +0300, Antony Pavlov wrote: > On Wed, 26 Jun 2019 09:12:02 +0200 > Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > Hi Antony, > > > > On Wed, Jun 19, 2019 at 01:49:16PM +0300, Antony Pavlov wrote: > > > Tap network interface initialization in sandbox > > > barebox leads to segfault under Debian Buster/Sid. > > > > > > The problem is that strcpy(dev, ifr.ifr_name) inside > > > tap_alloc() tries to alter read-only data passed > > > by tap_probe() and barebox receives SIGSEGV. > > > > > > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > > > --- > > > drivers/net/tap.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > > > index 1fbfa085b1..d7e32f4875 100644 > > > --- a/drivers/net/tap.c > > > +++ b/drivers/net/tap.c > > > @@ -14,7 +14,7 @@ > > > > > > struct tap_priv { > > > int fd; > > > - char *name; > > > + char name[128]; > > > }; > > > > > > static int tap_eth_send(struct eth_device *edev, void *packet, int length) > > > @@ -65,7 +65,7 @@ static int tap_probe(struct device_d *dev) > > > int ret = 0; > > > > > > priv = xzalloc(sizeof(struct tap_priv)); > > > - priv->name = "barebox"; > > > + strncpy(priv->name, "barebox", sizeof(priv->name)); > > > > > > priv->fd = tap_alloc(priv->name); > > > > Can we change the prototype of tap_alloc() to something like this: > > > > int tap_alloc(const char *name, int *fd, char **outname); > > > > outname would be an allocated string to be freed by the caller. > > There is one problem. > tap_alloc works in the sandbox "os domain" (glibc *alloc&free etc), > the caller works in the "barebox domain" (barebox *alloc&free). Oh, I didn't realize this. > > Can we just drop this outname? > nobody actually use it at the moment. Yes, let's just drop it. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-28 9:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-19 10:49 [RFC] sandbox: prevent segfault in tap_alloc() Antony Pavlov 2019-06-26 7:12 ` Sascha Hauer 2019-06-28 8:32 ` Antony Pavlov 2019-06-28 9:04 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox