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@xxxxxxxxx>
> Cc: Jann Horn <jannh@xxxxxxxxxx>
> Cc: Serge E. Hallyn <serge@xxxxxxxxxx>
> Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx>
Reviewed-by: James Morris <jamorris@xxxxxxxxxxxxxxxxxxx>
> ---
> 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. */
>
--
James Morris
<jmorris@xxxxxxxxx>
