From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56211) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAkt2-0007FP-2g for qemu-devel@nongnu.org; Tue, 04 Feb 2014 13:38:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WAkss-0002Qs-Lp for qemu-devel@nongnu.org; Tue, 04 Feb 2014 13:38:19 -0500 Received: from e23smtp07.au.ibm.com ([202.81.31.140]:48905) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WAiht-0008AA-79 for qemu-devel@nongnu.org; Tue, 04 Feb 2014 11:18:41 -0500 Received: from /spool/local by e23smtp07.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 5 Feb 2014 02:18:38 +1000 Received: from d23relay04.au.ibm.com (d23relay04.au.ibm.com [9.190.234.120]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 7078A3578053 for ; Wed, 5 Feb 2014 03:18:35 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s14FxGbG6357328 for ; Wed, 5 Feb 2014 02:59:16 +1100 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s14GIY4Y032638 for ; Wed, 5 Feb 2014 03:18:34 +1100 From: "Aneesh Kumar K.V" In-Reply-To: <52F0CD67.5070601@gmail.com> References: <52EF68CA.9060604@gmail.com> <20140203103429.GB10408@redhat.com> <52EF71DC.3000309@gmail.com> <52F0C8BA.7020709@gmail.com> <20140204110631.GD5632@redhat.com> <52F0CD67.5070601@gmail.com> Date: Tue, 04 Feb 2014 21:48:30 +0530 Message-ID: <87siry3l7t.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] hw/9pfs/virtio-9p-local.c: use snprintf() instead of sprintf() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Gang , "Daniel P. Berrange" Cc: QEMU Developers , aliguori@amazon.com Chen Gang writes: > On 02/04/2014 07:06 PM, Daniel P. Berrange wrote: >> On Tue, Feb 04, 2014 at 07:02:18PM +0800, Chen Gang wrote: >>> On 02/03/2014 06:39 PM, Chen Gang wrote: >>>> On 02/03/2014 06:34 PM, Daniel P. Berrange wrote: >>>>> On Mon, Feb 03, 2014 at 06:00:42PM +0800, Chen Gang wrote: >>>>>> We can not assume "'path' + 'ctx->fs_root'" must be less than MAX_PATH, >>>>>> so need use snprintf() instead of sprintf(). >>>>>> >>>>>> And also recommend to use ARRAY_SIZE instead of hard code macro for an >>>>>> array size in snprintf(). >>>>> >>>>> In the event that there is overflow this will cause the data to be >>>>> truncated, potentially causing QEMU to access the wrong file on the >>>>> host. Both snprintf and sprintf are really bad because of their >>>>> use of fixed buffers. Better to change it to g_strdup_printf which >>>>> dynamically allocates buffers. >>>>> >>> >>> After check the details, I guess we can not change to g_strdup_printf or >>> others (e.g. v9fs_string_*). >>> >>> v9fs need use "mkdir, remove ..." which have MAX_PATH limitation. So if >>> the combined path is longer than MAX_PATH, before it passes to "mkdir, >>> remove ...", it has to be truncated just like what rpath() has done. >> >> I don't believe you are correct there. Those functions should >> return "errno == ENAMETOOLONG - pathname was too long". The >> MAX_PATH constant is not even required to exist in POSIX, so >> I would not expect the spec to mandate anything about MAX_PATH >> in relation to those functions. >> > > So the original author of v9fs will use truncation instead of return > failure to upper users. That is a bug. The snprintf usage with PATH_MAX is to prevent buffer overflow and not to truncate. I guess we should fix path handling and propagate error correctly. -aneesh