All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] odd games with d_find_alias() in ceph_unlink()
@ 2025-01-06  4:57 Al Viro
  0 siblings, 0 replies; only message in thread
From: Al Viro @ 2025-01-06  4:57 UTC (permalink / raw)
  To: Xiubo Li; +Cc: Ilya Dryomov, Milind Changire, ceph-devel

	In 2827badaf8162 "ceph: check the cephx mds auth access for async dirop"
you've added the following chunk:

+       dn = d_find_alias(dir);
+       if (!dn) {
+               try_async = false;
+       } else {
+               path = ceph_mdsc_build_path(mdsc, dn, &pathlen, &pathbase, 0);
+               if (IS_ERR(path)) {
+                       try_async = false;
+                       err = 0;
+               } else {
+                       err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
+               }
+               ceph_mdsc_free_path(path, pathlen);
+               dput(dn);
+
+               /* For none EACCES cases will let the MDS do the mds auth check */
+               if (err == -EACCES) {
+                       return err;
+               } else if (err < 0) {
+                       try_async = false;
+                       err = 0;
+               }
+       }

What was that d_find_alias() for?  I mean, we are in ->unlink(); we'd
bloody well better have dentry->d_parent->d_inode == dir and dir locked
exclusive.  If that is _not_ true, we have a really big pile of problems
all over the place...  So your d_find_alias() is an obfuscated
	dn = dget(dentry->d_parent);
and even dget() is pointless here - dentry->d_parent is pinned, will stay
so and won't change.

What's wrong with simply

+       path = ceph_mdsc_build_path(mdsc, dentry->d_parent, &pathlen, &pathbase, 0);
+       if (IS_ERR(path)) {
+               try_async = false;
+               err = 0;
+       } else {
+               err = ceph_mds_check_access(mdsc, path, MAY_WRITE);
+       }
+       ceph_mdsc_free_path(path, pathlen);
+
+       /* For none EACCES cases will let the MDS do the mds auth check */
+       if (err == -EACCES) {
+               return err;
+       } else if (err < 0) {
+               try_async = false;
+               err = 0;
+       }

instead?  What am I missing here?  The same goes for the part in ceph_atomic_open()...

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-01-06  4:57 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-06  4:57 [RFC] odd games with d_find_alias() in ceph_unlink() Al Viro

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.