From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] tools/libxl: Don't read off the end of tinfo[] Date: Tue, 18 Feb 2014 18:14:33 +0000 Message-ID: <5303A309.3070900@citrix.com> References: <1392739145-24664-1-git-send-email-andrew.cooper3@citrix.com> <1392741544.23084.24.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1392741544.23084.24.camel@kazak.uk.xensource.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: Ian Campbell Cc: Dario Faggioli , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 18/02/14 16:39, Ian Campbell wrote: > On Tue, 2014-02-18 at 15:59 +0000, Andrew Cooper wrote: >> It is very common for BIOSes to advertise more cpus than are actually present >> on the system, and mark some of them as offline. This is what Xen does to >> allow for later CPU hotplug, and what BIOSes common to multiple different >> systems do to to save fully rewriting the MADT in memory. >> >> An excerpt from `xl info` might look like: >> >> ... >> nr_cpus : 2 >> max_cpu_id : 3 >> ... >> >> Which shows 4 CPUs in the MADT, but only 2 online (as this particular box is >> the dual-core rather than the quad-core SKU of its particular brand) >> >> Because of the way Xen exposes this information, a libxl_cputopology array is >> bounded by 'nr_cpus', while cpu bitmaps are bounded by 'max_cpu_id + 1'. >> >> The current libxl code has two places which erroneously assume that a >> libxl_cputopology array is as long as the number of bits found in a cpu >> bitmap, and valgrind complains: >> >> ==14961== Invalid read of size 4 >> ==14961== at 0x407AB7F: libxl__get_numa_candidate (libxl_numa.c:230) >> ==14961== by 0x407030B: libxl__build_pre (libxl_dom.c:167) >> ==14961== by 0x406246F: libxl__domain_build (libxl_create.c:371) >> ... >> ==14961== Address 0x4324788 is 8 bytes after a block of size 24 alloc'd >> ==14961== at 0x402669D: calloc (in/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) >> ==14961== by 0x4075BB9: libxl__zalloc (libxl_internal.c:83) >> ==14961== by 0x4052F87: libxl_get_cpu_topology (libxl.c:4408) >> ==14961== by 0x407A899: libxl__get_numa_candidate (libxl_numa.c:342) >> ... >> >> Signed-off-by: Andrew Cooper > Acked-by: Ian Campbell > > Unless someone argues otherwise this is going into my 4.5 pile. > > If 4.4 gets delayed, and patches such as the RTC series are re-up for consideration, then this should also be considered. If not, then 4.5 is fine, along with a backport to 4.4.x and 4.3.x. ~Andrew