From owner-freebsd-fs@freebsd.org Sun May 21 12:31:26 2017 Return-Path: Delivered-To: freebsd-fs@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F3A6DD6BEBB; Sun, 21 May 2017 12:31:25 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 80A761149; Sun, 21 May 2017 12:31:25 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id v4LCVIeJ019814 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Sun, 21 May 2017 15:31:19 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua v4LCVIeJ019814 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id v4LCVINU019813; Sun, 21 May 2017 15:31:18 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 21 May 2017 15:31:18 +0300 From: Konstantin Belousov To: Jilles Tjoelker Cc: freebsd-current@freebsd.org, freebsd-fs@freebsd.org, freebsd-ports@freebsd.org, emaste@freebsd.org, Kirk McKusick Subject: Re: 64-bit inodes (ino64) Status Update and Call for Testing Message-ID: <20170521123118.GH1622@kib.kiev.ua> References: <20170420194314.GI1788@kib.kiev.ua> <20170521121456.GA21613@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170521121456.GA21613@stack.nl> User-Agent: Mutt/1.8.2 (2017-04-18) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on tom.home X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 May 2017 12:31:26 -0000 On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote: > We have another type in this area which is too small in some situations: > uint8_t for struct dirent.d_namlen. For filesystems that store filenames > as upto 255 UTF-16 code units, the name to be stored in d_name may be > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code > currently handles this by returning the short (8.3) name, but this name > may not be present or usable, leaving the file inaccessible. > > Actually allowing longer names seems too complicated to add to the ino64 > change, but changing d_namlen to uint16_t (using d_pad0 space) and > skipping entries with d_namlen > 255 in libc may be helpful. > > Note that applications using the deprecated readdir_r() will not be able > to read such long names, since the API does not allow specifying that a > larger buffer has been provided. (This could be avoided by making struct > dirent.d_name 766 bytes long instead of 256.) > > Unfortunately, the existence of readdir_r() also prevents changing > struct dirent.d_name to the more correct flexible array. Yes, changing the size of d_name at this stage of the project is out of question. My reading of your proposal is that we should extend the size of d_namlen to uint16_t, am I right ? Should we go to 32bit directly then, perhaps ? I did not committed the change below, nor did I tested or even build it. diff --git a/lib/libc/gen/readdir-compat11.c b/lib/libc/gen/readdir-compat11.c index 1c52f563c75..18d85adaa63 100644 --- a/lib/libc/gen/readdir-compat11.c +++ b/lib/libc/gen/readdir-compat11.c @@ -41,6 +41,7 @@ __FBSDID("$FreeBSD$"); #define _WANT_FREEBSD11_DIRENT #include #include +#include #include #include #include @@ -53,10 +54,12 @@ __FBSDID("$FreeBSD$"); #include "gen-compat.h" -static void +static bool freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct dirent *srcdp) { + if (srcdp->d_namelen >= sizeof(dstdp->d_name)) + return (false); dstdp->d_type = srcdp->d_type; dstdp->d_namlen = srcdp->d_namlen; dstdp->d_fileno = srcdp->d_fileno; /* truncate */ @@ -65,6 +68,7 @@ freebsd11_cvtdirent(struct freebsd11_dirent *dstdp, struct dirent *srcdp) bzero(dstdp->d_name + dstdp->d_namlen, dstdp->d_reclen - offsetof(struct freebsd11_dirent, d_name) - dstdp->d_namlen); + return (true); } struct freebsd11_dirent * @@ -80,8 +84,10 @@ freebsd11_readdir(DIR *dirp) if (dirp->dd_compat_de == NULL) dirp->dd_compat_de = malloc(sizeof(struct freebsd11_dirent)); - freebsd11_cvtdirent(dirp->dd_compat_de, dp); - dstdp = dirp->dd_compat_de; + if (freebsd11_cvtdirent(dirp->dd_compat_de, dp)) + dstdp = dirp->dd_compat_de; + else + dstdp = NULL; } else dstdp = NULL; if (__isthreaded) @@ -101,8 +107,10 @@ freebsd11_readdir_r(DIR *dirp, struct freebsd11_dirent *entry, if (error != 0) return (error); if (xresult != NULL) { - freebsd11_cvtdirent(entry, &xentry); - *result = entry; + if (freebsd11_cvtdirent(entry, &xentry)) + *result = entry; + else + *result = NULL; } else *result = NULL; return (0); diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c index 784af836aee..27b2635030d 100644 --- a/sys/kern/vfs_syscalls.c +++ b/sys/kern/vfs_syscalls.c @@ -3733,7 +3733,8 @@ freebsd11_kern_getdirentries(struct thread *td, int fd, char *ubuf, u_int count, if (dp->d_reclen == 0) break; MPASS(dp->d_reclen >= _GENERIC_DIRLEN(0)); - /* dp->d_namlen <= sizeof(dstdp.d_name) - 1 always */ + if (dp->d_namlen >= sizeof(dstdp.d_name)) + continue; dstdp.d_type = dp->d_type; dstdp.d_namlen = dp->d_namlen; dstdp.d_fileno = dp->d_fileno; /* truncate */ diff --git a/sys/sys/dirent.h b/sys/sys/dirent.h index 341855d0530..691c4e8f90f 100644 --- a/sys/sys/dirent.h +++ b/sys/sys/dirent.h @@ -67,8 +67,9 @@ struct dirent { off_t d_off; /* directory offset of entry */ __uint16_t d_reclen; /* length of this record */ __uint8_t d_type; /* file type, see below */ - __uint8_t d_namlen; /* length of string in d_name */ - __uint32_t d_pad0; + __uint8_t d_pad0 + __uint16_t d_namlen; /* length of string in d_name */ + __uint16_t d_pad1; #if __BSD_VISIBLE #define MAXNAMLEN 255 char d_name[MAXNAMLEN + 1]; /* name must be no longer than this */