On Wed, Feb 12, 2025 at 03:16:08AM +0000, Al Viro wrote:
> On Fri, Feb 07, 2025 at 02:36:48PM +1100, NeilBrown wrote:
> > @@ -1690,6 +1692,15 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name,
> > dput(dentry);
> > dentry = old;
> > }
> > +found:
>
> ... and if ->lookup() returns an error, this will blow up (as bot has just
> reported).
>
> > + if (d_is_negative(dentry) && !(flags & LOOKUP_CREATE)) {
> > + dput(dentry);
> > + return ERR_PTR(-ENOENT);
> > + }
> > + if (d_is_positive(dentry) && (flags & LOOKUP_EXCL)) {
> > + dput(dentry);
> > + return ERR_PTR(-EEXIST);
> > + }
>
>
> > @@ -4077,27 +4084,13 @@ static struct dentry *filename_create(int dfd, struct filename *name,
> > * '/', and a directory wasn't requested.
> > */
> > if (last.name[last.len] && !want_dir)
> > - create_flags = 0;
> > + create_flags &= ~LOOKUP_CREATE;
>
> See the patch I've posted in earlier thread; the entire "strip LOOKUP_CREATE"
> thing is wrong.
On top of mainline that's
filename_create(): don't force handling trailing slashes into the common path
Only mkdir accepts pathnames that end with / - anything like mknod() (symlink(),
etc.) always fails on those. Don't try to force that the common codepath -
all we are doing is a lookup and check for existence to determine which
error should it be. Do that before bothering with mnt_want_write(), etc.;
as far as underlying filesystem is concerned it's just a lookup. Simplifies
the normal codepath and kills the lookup intent dependency on more than
the call site.
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
diff --git a/fs/namei.c b/fs/namei.c
index 3ab9440c5b93..6189e54f767a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4054,13 +4054,13 @@ static struct dentry *filename_create(int dfd, struct filename *name,
struct dentry *dentry = ERR_PTR(-EEXIST);
struct qstr last;
bool want_dir = lookup_flags & LOOKUP_DIRECTORY;
- unsigned int reval_flag = lookup_flags & LOOKUP_REVAL;
- unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL;
int type;
int err2;
int error;
- error = filename_parentat(dfd, name, reval_flag, path, &last, &type);
+ lookup_flags &= LOOKUP_REVAL;
+
+ error = filename_parentat(dfd, name, lookup_flags, path, &last, &type);
if (error)
return ERR_PTR(error);
@@ -4070,18 +4070,28 @@ static struct dentry *filename_create(int dfd, struct filename *name,
*/
if (unlikely(type != LAST_NORM))
goto out;
+ /*
+ * mkdir foo/bar/ is OK, but for anything else a slash in the end
+ * is always an error; the only question is which one.
+ */
+ if (unlikely(last.name[last.len] && !want_dir)) {
+ dentry = lookup_dcache(&last, path->dentry, lookup_flags);
+ if (!dentry)
+ dentry = lookup_slow(&last, path->dentry, lookup_flags);
+ if (!IS_ERR(dentry)) {
+ error = d_is_positive(dentry) ? -EEXIST : -ENOENT;
+ dput(dentry);
+ dentry = ERR_PTR(error);
+ }
+ goto out;
+ }
/* don't fail immediately if it's r/o, at least try to report other errors */
err2 = mnt_want_write(path->mnt);
- /*
- * Do the final lookup. Suppress 'create' if there is a trailing
- * '/', and a directory wasn't requested.
- */
- if (last.name[last.len] && !want_dir)
- create_flags = 0;
+ /* do the final lookup */
inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT);
dentry = lookup_one_qstr_excl(&last, path->dentry,
- reval_flag | create_flags);
+ lookup_flags | LOOKUP_CREATE | LOOKUP_EXCL);
if (IS_ERR(dentry))
goto unlock;
@@ -4089,16 +4099,6 @@ static struct dentry *filename_create(int dfd, struct filename *name,
if (d_is_positive(dentry))
goto fail;
- /*
- * Special case - lookup gave negative, but... we had foo/bar/
- * From the vfs_mknod() POV we just have a negative dentry -
- * all is fine. Let's be bastards - you had / on the end, you've
- * been asking for (non-existent) directory. -ENOENT for you.
- */
- if (unlikely(!create_flags)) {
- error = -ENOENT;
- goto fail;
- }
if (unlikely(err2)) {
error = err2;
goto fail;
