[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/4] minios: correct char array allocation for xenbus paths
On Fri, 2013-09-06 at 12:52 -0700, Matt Wilson wrote: > From: Matt Wilson <msw@xxxxxxxxxx> > > The char arrays used to hold xenbus paths have historically been > allocated by manually counting the length longest string constants > included in constructing the path. This has led to improperly sized > buffers, both too large (with little consequence) and too small (which > obviously causes problems). This patch corrects the instances where > the length was incorrectly calculated by using strlen() on the longest > string constant used in building a xenbus path. > > A follow-on clean-up patch will change all instances to use strlen(). > > Signed-off-by: Ben Cressey <bcressey@xxxxxxxxxx> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> > Cc: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx> > [msw: split this patch from a larger patch from Ben, reworked to use > strlen()] > Signed-off-by: Matt Wilson <msw@xxxxxxxxxx> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx> > --- > extras/mini-os/blkfront.c | 2 +- > extras/mini-os/console/xenbus.c | 2 +- > extras/mini-os/fbfront.c | 10 +++++----- > extras/mini-os/netfront.c | 2 +- > extras/mini-os/pcifront.c | 7 +++++-- > 5 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/extras/mini-os/blkfront.c b/extras/mini-os/blkfront.c > index f4283a9..70976f5 100644 > --- a/extras/mini-os/blkfront.c > +++ b/extras/mini-os/blkfront.c > @@ -254,7 +254,7 @@ void shutdown_blkfront(struct blkfront_dev *dev) > XenbusState state; > > char path[strlen(dev->backend) + 1 + 5 + 1]; > - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; > + char nodename[strlen(dev->nodename) + strlen("/event-channel") + 1]; > > blkfront_sync(dev); > > diff --git a/extras/mini-os/console/xenbus.c b/extras/mini-os/console/xenbus.c > index e65baf7..1ecfcc5 100644 > --- a/extras/mini-os/console/xenbus.c > +++ b/extras/mini-os/console/xenbus.c > @@ -158,7 +158,7 @@ done: > > { > XenbusState state; > - char path[strlen(dev->backend) + 1 + 19 + 1]; > + char path[strlen(dev->backend) + strlen("/state") + 1]; > snprintf(path, sizeof(path), "%s/state", dev->backend); > > xenbus_watch_path_token(XBT_NIL, path, path, &dev->events); > diff --git a/extras/mini-os/fbfront.c b/extras/mini-os/fbfront.c > index 54a5e67..6eddb3c 100644 > --- a/extras/mini-os/fbfront.c > +++ b/extras/mini-os/fbfront.c > @@ -158,8 +158,8 @@ done: > > { > XenbusState state; > - char path[strlen(dev->backend) + 1 + 6 + 1]; > - char frontpath[strlen(nodename) + 1 + 6 + 1]; > + char path[strlen(dev->backend) + strlen("/state") + 1]; > + char frontpath[strlen(nodename) + strlen("/state") + 1]; > > snprintf(path, sizeof(path), "%s/state", dev->backend); > > @@ -240,7 +240,7 @@ void shutdown_kbdfront(struct kbdfront_dev *dev) > XenbusState state; > > char path[strlen(dev->backend) + 1 + 5 + 1]; > - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; > + char nodename[strlen(dev->nodename) + strlen("/request-abs-pointer") + > 1]; > > printk("close kbd: backend at %s\n",dev->backend); > > @@ -521,7 +521,7 @@ done: > { > XenbusState state; > char path[strlen(dev->backend) + 1 + 14 + 1]; > - char frontpath[strlen(nodename) + 1 + 6 + 1]; > + char frontpath[strlen(nodename) + strlen("/state") + 1]; > > snprintf(path, sizeof(path), "%s/state", dev->backend); > > @@ -632,7 +632,7 @@ void shutdown_fbfront(struct fbfront_dev *dev) > XenbusState state; > > char path[strlen(dev->backend) + 1 + 5 + 1]; > - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; > + char nodename[strlen(dev->nodename) + strlen("/feature-update") + 1]; > > printk("close fb: backend at %s\n",dev->backend); > > diff --git a/extras/mini-os/netfront.c b/extras/mini-os/netfront.c > index 6fa68a2..ddf56ea 100644 > --- a/extras/mini-os/netfront.c > +++ b/extras/mini-os/netfront.c > @@ -508,7 +508,7 @@ void shutdown_netfront(struct netfront_dev *dev) > XenbusState state; > > char path[strlen(dev->backend) + 1 + 5 + 1]; > - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; > + char nodename[strlen(dev->nodename) + strlen("/request-rx-copy") + 1]; > > printk("close network: backend at %s\n",dev->backend); > > diff --git a/extras/mini-os/pcifront.c b/extras/mini-os/pcifront.c > index bbe21e0..f9ae768 100644 > --- a/extras/mini-os/pcifront.c > +++ b/extras/mini-os/pcifront.c > @@ -323,7 +323,7 @@ void shutdown_pcifront(struct pcifront_dev *dev) > XenbusState state; > > char path[strlen(dev->backend) + 1 + 5 + 1]; > - char nodename[strlen(dev->nodename) + 1 + 5 + 1]; > + char nodename[strlen(dev->nodename) + strlen("/event-channel") + 1]; > > printk("close pci: backend at %s\n",dev->backend); > > @@ -379,7 +379,10 @@ int pcifront_physical_to_virtual (struct pcifront_dev > *dev, > unsigned int *slot, > unsigned long *fun) > { > - char path[strlen(dev->backend) + 1 + 5 + 10 + 1]; > + /* FIXME: the buffer sizing is a little lazy here. 10 extra bytes > + should be enough to hold the paths we need to construct, even > + if the number of devices is large */ > + char path[strlen(dev->backend) + strlen("/num_devs") + 10 + 1]; > int i, n; > char *s, *msg = NULL; > unsigned int dom1, bus1, slot1, fun1; _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |