Chen Gang <gang.chen.5...@gmail.com> 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
