From: Al Viro <viro@zeniv.linux.org.uk>
To: Xiubo Li <xiubli@redhat.com>
Cc: Ilya Dryomov <idryomov@gmail.com>,
Milind Changire <mchangir@redhat.com>,
ceph-devel@vger.kernel.org
Subject: [RFC] odd games with d_find_alias() in ceph_unlink()
Date: Mon, 6 Jan 2025 04:57:24 +0000 [thread overview]
Message-ID: <20250106045724.GM1977892@ZenIV> (raw)
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()...
reply other threads:[~2025-01-06 4:57 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250106045724.GM1977892@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=ceph-devel@vger.kernel.org \
--cc=idryomov@gmail.com \
--cc=mchangir@redhat.com \
--cc=xiubli@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox