| Message ID | 20201111213442.434639-3-mic@digikod.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Landlock fixes | expand |
On Wed, 11 Nov 2020, Mickaël Salaün wrote: > Improve comments and make get_inode_object() more readable. The kfree() > call is correct but we should mimimize as much as possible lock windows. > > Cc: James Morris <jmorris@namei.org> > Cc: Jann Horn <jannh@google.com> > Cc: Serge E. Hallyn <serge@hallyn.com> > Signed-off-by: Mickaël Salaün <mic@digikod.net> Reviewed-by: James Morris <jamorris@linux.microsoft.com> > --- > security/landlock/fs.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > index b67c821bb40b..33fc7ae17c7f 100644 > --- a/security/landlock/fs.c > +++ b/security/landlock/fs.c > @@ -85,8 +85,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > return object; > } > /* > - * We're racing with release_inode(), the object is going away. > - * Wait for release_inode(), then retry. > + * We are racing with release_inode(), the object is going > + * away. Wait for release_inode(), then retry. > */ > spin_lock(&object->lock); > spin_unlock(&object->lock); > @@ -107,21 +107,21 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > lockdep_is_held(&inode->i_lock)); > if (unlikely(object)) { > /* Someone else just created the object, bail out and retry. */ > - kfree(new_object); > spin_unlock(&inode->i_lock); > + kfree(new_object); > > rcu_read_lock(); > goto retry; > - } else { > - rcu_assign_pointer(inode_sec->object, new_object); > - /* > - * @inode will be released by hook_sb_delete() on its > - * superblock shutdown. > - */ > - ihold(inode); > - spin_unlock(&inode->i_lock); > - return new_object; > } > + > + rcu_assign_pointer(inode_sec->object, new_object); > + /* > + * @inode will be released by hook_sb_delete() on its superblock > + * shutdown. > + */ > + ihold(inode); > + spin_unlock(&inode->i_lock); > + return new_object; > } > > /* All access rights which can be tied to files. */ >
diff --git a/security/landlock/fs.c b/security/landlock/fs.c index b67c821bb40b..33fc7ae17c7f 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -85,8 +85,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode) return object; } /* - * We're racing with release_inode(), the object is going away. - * Wait for release_inode(), then retry. + * We are racing with release_inode(), the object is going + * away. Wait for release_inode(), then retry. */ spin_lock(&object->lock); spin_unlock(&object->lock); @@ -107,21 +107,21 @@ static struct landlock_object *get_inode_object(struct inode *const inode) lockdep_is_held(&inode->i_lock)); if (unlikely(object)) { /* Someone else just created the object, bail out and retry. */ - kfree(new_object); spin_unlock(&inode->i_lock); + kfree(new_object); rcu_read_lock(); goto retry; - } else { - rcu_assign_pointer(inode_sec->object, new_object); - /* - * @inode will be released by hook_sb_delete() on its - * superblock shutdown. - */ - ihold(inode); - spin_unlock(&inode->i_lock); - return new_object; } + + rcu_assign_pointer(inode_sec->object, new_object); + /* + * @inode will be released by hook_sb_delete() on its superblock + * shutdown. + */ + ihold(inode); + spin_unlock(&inode->i_lock); + return new_object; } /* All access rights which can be tied to files. */
Improve comments and make get_inode_object() more readable. The kfree() call is correct but we should mimimize as much as possible lock windows. Cc: James Morris <jmorris@namei.org> Cc: Jann Horn <jannh@google.com> Cc: Serge E. Hallyn <serge@hallyn.com> Signed-off-by: Mickaël Salaün <mic@digikod.net> --- security/landlock/fs.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)