From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39511) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WApfT-0003JY-Vx for qemu-devel@nongnu.org; Tue, 04 Feb 2014 18:44:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WApfH-00040W-K2 for qemu-devel@nongnu.org; Tue, 04 Feb 2014 18:44:39 -0500 Received: from mail-pa0-x22e.google.com ([2607:f8b0:400e:c03::22e]:38734) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WApfH-00040S-Bt for qemu-devel@nongnu.org; Tue, 04 Feb 2014 18:44:27 -0500 Received: by mail-pa0-f46.google.com with SMTP id rd3so9097209pab.5 for ; Tue, 04 Feb 2014 15:44:26 -0800 (PST) Message-ID: <52F17B5E.1050602@gmail.com> Date: Wed, 05 Feb 2014 07:44:30 +0800 From: Chen Gang MIME-Version: 1.0 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> <87siry3l7t.fsf@linux.vnet.ibm.com> In-Reply-To: <87siry3l7t.fsf@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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: "Aneesh Kumar K.V" Cc: QEMU Developers , aliguori@amazon.com On 02/05/2014 12:18 AM, Aneesh Kumar K.V wrote: > 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 > OK, thank you for your opinion and confirmation. I will/should send patch v2 for it (use 'unlimited' path and propagate error correctly). And excuse me, I have no enough time to focus on it, so I plan to send patch v2 for reviewing within this month (2014-02-28). If we can not bear this time point, please help send patch for it, thanks. And also excuse me, I am a newbie for 9pfs, also a newbie for qemu (I found it by reading source code), the patch v2 needs a test for 9pfs, so welcome any suggestions/informations about 9pfs test. Thanks -- Chen Gang Open, share and attitude like air, water and life which God blessed