From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boqun Feng Subject: Is it OK to export symbols 'getname' and 'putname'? Date: Fri, 17 Apr 2015 20:35:30 +0800 Message-ID: <20150417123530.GA4249@fixme-laptop.cn.ibm.com> References: <1427309152-25129-1-git-send-email-boqun.feng@gmail.com> <20150329042744.GA1477@fixme-laptop> <20150407083826.GA1048@fixme-laptop.cn.ibm.com> <20150411235655.GK889@ZenIV.linux.org.uk> <20150412011318.GL889@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SUOF0GtieIMvvwua" Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Al Viro Return-path: Content-Disposition: inline In-Reply-To: <20150412011318.GL889@ZenIV.linux.org.uk> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org --SUOF0GtieIMvvwua Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Al, On Sun, Apr 12, 2015 at 02:13:18AM +0100, Al Viro wrote: >=20 > BTW, looking at the __getname() callers... Lustre one sure as hell looks > bogus: > char *tmp =3D __getname(); >=20 > if (!tmp) > return ERR_PTR(-ENOMEM); >=20 > len =3D strncpy_from_user(tmp, filename, PATH_MAX); > if (len =3D=3D 0) > ret =3D -ENOENT; > else if (len > PATH_MAX) > ret =3D -ENAMETOOLONG; >=20 > if (ret) { > __putname(tmp); > tmp =3D ERR_PTR(ret); > } > return tmp; >=20 > Note that > * strncpy_from_user(p, u, n) can return a negative (-EFAULT) > * strncpy_from_user(p, u, n) cannot return a positive greater than > n. In case of missing NUL among the n bytes starting at u (and no faults > accessing those) we get exactly n bytes copied and n returned. In case > when NUL is there, we copy everything up to and including that NUL and > return number of non-NUL bytes copied. >=20 > IOW, these failure cases had never been tested. Name being too long ends= up > with non-NUL-terminated array of characters returned, and the very first > caller of ll_getname() feeds it to strlen(). Fault ends up with uninitia= lized > array... >=20 > AFAICS, the damn thing should just use getname() and quit reinventing the > wheel, badly. >=20 I'm trying to clean that part of code you mentioned, and I found I have to export the symbols 'getname' and 'putname' as follow to replace that __getname() caller: diff --git a/drivers/staging/lustre/lustre/llite/dir.c b/drivers/staging/lu= stre/lustre/llite/dir.c index a182019..014f51a 100644 --- a/drivers/staging/lustre/lustre/llite/dir.c +++ b/drivers/staging/lustre/lustre/llite/dir.c @@ -1216,29 +1216,8 @@ out: return rc; } =20 -static char * -ll_getname(const char __user *filename) -{ - int ret =3D 0, len; - char *tmp =3D __getname(); - - if (!tmp) - return ERR_PTR(-ENOMEM); - - len =3D strncpy_from_user(tmp, filename, PATH_MAX); - if (len =3D=3D 0) - ret =3D -ENOENT; - else if (len > PATH_MAX) - ret =3D -ENAMETOOLONG; - - if (ret) { - __putname(tmp); - tmp =3D ERR_PTR(ret); - } - return tmp; -} - -#define ll_putname(filename) __putname(filename) +#define ll_getname(filename) getname(filename) +#define ll_putname(name) putname(name) =20 static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned lon= g arg) { @@ -1441,6 +1420,7 @@ free_lmv: return rc; } case LL_IOC_REMOVE_ENTRY: { + struct filename *name =3D NULL; char *filename =3D NULL; int namelen =3D 0; int rc; @@ -1453,20 +1433,17 @@ free_lmv: if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_LVB_TYPE)) return -ENOTSUPP; =20 - filename =3D ll_getname((const char *)arg); - if (IS_ERR(filename)) - return PTR_ERR(filename); + name =3D ll_getname((const char *)arg); + if (IS_ERR(name)) + return PTR_ERR(name); =20 + filename =3D name->name; namelen =3D strlen(filename); - if (namelen < 1) { - rc =3D -EINVAL; - goto out_rmdir; - } =20 rc =3D ll_rmdir_entry(inode, filename, namelen); out_rmdir: - if (filename) - ll_putname(filename); + if (name) + ll_putname(name); return rc; } case LL_IOC_LOV_SWAP_LAYOUTS: @@ -1481,15 +1458,17 @@ out_rmdir: struct lov_user_md *lump; struct lov_mds_md *lmm =3D NULL; struct mdt_body *body; + struct filename *name; char *filename =3D NULL; int lmmsize; =20 if (cmd =3D=3D IOC_MDC_GETFILEINFO || cmd =3D=3D IOC_MDC_GETFILESTRIPE) { - filename =3D ll_getname((const char *)arg); - if (IS_ERR(filename)) - return PTR_ERR(filename); + name =3D ll_getname((const char *)arg); + if (IS_ERR(name)) + return PTR_ERR(name); =20 + filename =3D name->name; rc =3D ll_lov_getstripe_ea_info(inode, filename, &lmm, &lmmsize, &request); } else { diff --git a/fs/namei.c b/fs/namei.c index ffab2e0..7a0948c 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -205,6 +205,7 @@ getname(const char __user * filename) { return getname_flags(filename, 0, NULL); } +EXPORT_SYMBOL(getname); =20 struct filename * getname_kernel(const char * filename) @@ -254,6 +255,7 @@ void putname(struct filename *name) } else __putname(name); } +EXPORT_SYMBOL(putname); =20 static int check_acl(struct inode *inode, int mask) { Is that a good idea to export these symbols, given that lustre may be the only user?=20 Looking forward to your insight. Thanks, Boqun --SUOF0GtieIMvvwua Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJVMP4NAAoJEEl56MO1B/q4Y+QH/ibgBvYK0aC7+MGzOaFaN2BJ CbR/jCZ2eCjy+mvTtfWy6WLLr21KlLJBRGUygk02BO5SWGDOx6ZdfowgBi6P8hqC VDWy70hcvD9Osl+Nm6pvVJdP8qL89rmX7nD8Sc4OgSwKQJgK1mHFaQwoBodKUhEn wbtPr2spbC311KoAM2755jbiH4/tTi2xkgcFYH3hYwmxRCoUqEVe/omiORQJcS9h FWgiZy8AVHmByBP6wM1ApvXoH+/Ea7K6kNYuvCOvH+SUBMWAGOR6PzWf8Uijjkc3 3WxcSHbjxyzdxzE9nHawNx135Uam+3f54P7Nq33YWjYwPZ7RzyDmQWOSKpLx8rI= =orEN -----END PGP SIGNATURE----- --SUOF0GtieIMvvwua--