From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v3 1/2] libxc/xentrace: Replace xc_tbuf_set_cpu_mask with CPU mask with xc_cpumap_t instead of uint32_t Date: Tue, 24 Jun 2014 15:35:53 +0100 Message-ID: <53A98CC9.2050401@eu.citrix.com> References: <1403292831-3143-1-git-send-email-konrad.wilk@oracle.com> <1403292831-3143-2-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1403292831-3143-2-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk , xen-devel@lists.xen.org, ian.campbell@citrix.com, ian.jackson@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 06/20/2014 08:33 PM, Konrad Rzeszutek Wilk wrote: > @@ -906,6 +949,23 @@ static int parse_evtmask(char *arg) > return 0; > } > > +static int parse_cpumask(const char *arg) > +{ > + xc_cpumap_t map; > + uint32_t v, i; > + > + map = malloc(sizeof(uint32_t)); > + if ( !map ) > + return -ENOMEM; > + > + v = argtol(arg, 0); > + for ( i = 0; i < sizeof(uint32_t); i++ ) > + map[i] = (v >> (i * 8)) & 0xff; > + > + opts.cpu_mask = map; Sorry for not noticing this befori. If I'm reading this right, if someone sets the cpumask as a hex, then opts.cpu_mask will point to an area of memory only 32 bits long. However, up in set_cpu_mask(), it always calls xc_tbuf_set_cpu_mask() with bits equal to xc_get_max_cpus(). On systems with more than 32 logical cpus, won't this cause a buffer overrun? Should you be calling xc_cpumap_alloc() here instead of malloc()? > @@ -937,7 +997,12 @@ static void parse_args(int argc, char **argv) > break; > > case 'c': /* set new cpu mask for filtering*/ > - opts.cpu_mask = argtol(optarg, 0); > + /* Set opts.cpu_mask later as we don't have 'xch' set yet. */ > + if ( parse_cpumask(optarg) ) Nit: You seem to be parsing it now. :-) -George