From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miklos Szeredi Subject: Re: [PATCH v4] ovl: Check link ability between upperdir and workdir Date: Fri, 19 Jan 2018 17:25:49 +0100 Message-ID: References: <1513735754-40567-1-git-send-email-cgxu519@icloud.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-qt0-f193.google.com ([209.85.216.193]:45609 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755944AbeASQZu (ORCPT ); Fri, 19 Jan 2018 11:25:50 -0500 Received: by mail-qt0-f193.google.com with SMTP id x27so5096338qtm.12 for ; Fri, 19 Jan 2018 08:25:50 -0800 (PST) In-Reply-To: <1513735754-40567-1-git-send-email-cgxu519@icloud.com> Sender: linux-unionfs-owner@vger.kernel.org List-Id: linux-unionfs@vger.kernel.org To: Chengguang Xu Cc: Amir Goldstein , Vivek Goyal , overlayfs On Wed, Dec 20, 2017 at 3:09 AM, Chengguang Xu wrote: > Inspired by encountering unexpected write error with > error code -EXDEV when upperdir and workdir having > different project quotas. The reason of the error is > project quota asks files in it strictly inherit > project id of it=E2=80=99s own, so rename/link operations > between those directories will fail and it may cause > failure during copy-up/index-link operations in overlayfs. > > So if upper filesystem supports O_TMPFILE, try to check > link ability between upperdir and workdir and if check > fails, print a proper warning message to indicate potential > possibility of write failure. > > The failure of the check does not directly lead to > read-only mounting because in some use cases may only > write to upperdir and do not modify anything in lowerdirs. > > Signed-off-by: Chengguang Xu > --- > Changes since v3: > - Add more detail comment about motivation to changelog. > > Changes since v2: > - Add comment about motivation. > - Modify warning message. > > Changes since v1: > - Check link ablity between upperdir and workdir instead of checking > project ids of upperdir and workdir. > - Print a warning message instead of falling to readonly mode. > > fs/overlayfs/super.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 76440fe..7d6bd82 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -929,7 +929,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct p= ath *upperpath) > > static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath) > { > - struct dentry *temp; > + struct dentry *uppertemp, *worktemp; > int err; > > ofs->workdir =3D ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false)= ; > @@ -953,12 +953,28 @@ static int ovl_make_workdir(struct ovl_fs *ofs, str= uct path *workpath) > if (!err) > pr_warn("overlayfs: upper fs needs to support d_type.\n")= ; > > - /* Check if upper/work fs supports O_TMPFILE */ > - temp =3D ovl_do_tmpfile(ofs->workdir, S_IFREG | 0); > - ofs->tmpfile =3D !IS_ERR(temp); > - if (ofs->tmpfile) > - dput(temp); > - else > + /* > + * Check if upper/work fs supports O_TMPFILE and if support, chec= k > + * link ability between upperdir and workdir, this is inspired by > + * encountering unexpected write error when upperdir and workdir > + * having different project ids. > + */ > + uppertemp =3D ovl_do_tmpfile(ofs->upper_mnt->mnt_root, S_IFREG | = 0); > + ofs->tmpfile =3D !IS_ERR(uppertemp); > + if (ofs->tmpfile) { > + worktemp =3D ovl_lookup_temp(ofs->workdir); Have you tested this? Because there's a warning at the top of lookup_one_len() that triggers because of no locking of parent. I can add the locking, it's trivial, but I'd still like you to test this on your setup, if it actually does what you promise it does. Thanks, Miklos