| Message ID | 20241124-work-cred-v1-21-f352241c3970@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cred: rework {override,revert}_creds() | expand |
On Sun, Nov 24, 2024 at 02:44:07PM +0100, Christian Brauner wrote: > No need for the extra reference count bump. > > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > fs/smb/server/smb_common.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c > index f1d770a214c8b2c7d7dd4083ef57c7130bbce52c..a3f96804f84f03c22376769dffdf60cd66f5e3d2 100644 > --- a/fs/smb/server/smb_common.c > +++ b/fs/smb/server/smb_common.c > @@ -780,7 +780,7 @@ int __ksmbd_override_fsids(struct ksmbd_work *work, > cred->cap_effective = cap_drop_fs_set(cred->cap_effective); > > WARN_ON(work->saved_cred); > - work->saved_cred = override_creds(get_new_cred(cred)); > + work->saved_cred = override_creds(cred); > if (!work->saved_cred) { > abort_creds(cred); > return -EINVAL; Won't that leave a dangling pointer?
On Sun, Nov 24, 2024 at 06:37:43PM +0000, Al Viro wrote: > On Sun, Nov 24, 2024 at 02:44:07PM +0100, Christian Brauner wrote: > > No need for the extra reference count bump. > > > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > --- > > fs/smb/server/smb_common.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c > > index f1d770a214c8b2c7d7dd4083ef57c7130bbce52c..a3f96804f84f03c22376769dffdf60cd66f5e3d2 100644 > > --- a/fs/smb/server/smb_common.c > > +++ b/fs/smb/server/smb_common.c > > @@ -780,7 +780,7 @@ int __ksmbd_override_fsids(struct ksmbd_work *work, > > cred->cap_effective = cap_drop_fs_set(cred->cap_effective); > > > > WARN_ON(work->saved_cred); > > - work->saved_cred = override_creds(get_new_cred(cred)); > > + work->saved_cred = override_creds(cred); > > if (!work->saved_cred) { > > abort_creds(cred); > > return -EINVAL; > > Won't that leave a dangling pointer? Afaict, the whole check doesn't make sense because I don't see how override_creds() could be called on a task with current->cred == NULL. There's no way to opt out of having current->cred set.
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c index f1d770a214c8b2c7d7dd4083ef57c7130bbce52c..a3f96804f84f03c22376769dffdf60cd66f5e3d2 100644 --- a/fs/smb/server/smb_common.c +++ b/fs/smb/server/smb_common.c @@ -780,7 +780,7 @@ int __ksmbd_override_fsids(struct ksmbd_work *work, cred->cap_effective = cap_drop_fs_set(cred->cap_effective); WARN_ON(work->saved_cred); - work->saved_cred = override_creds(get_new_cred(cred)); + work->saved_cred = override_creds(cred); if (!work->saved_cred) { abort_creds(cred); return -EINVAL; @@ -799,9 +799,7 @@ void ksmbd_revert_fsids(struct ksmbd_work *work) WARN_ON(!work->saved_cred); - cred = current_cred(); put_cred(revert_creds(work->saved_cred)); - put_cred(cred); work->saved_cred = NULL; }
No need for the extra reference count bump. Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/smb/server/smb_common.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)