From: Al Viro <viro@zeniv.linux.org.uk>
To: Kees Cook <keescook@chromium.org>
Cc: "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
Tony Luck <tony.luck@intel.com>,
linux-hardening@vger.kernel.org,
Christian Brauner <brauner@kernel.org>,
"Peter Zijlstra (Intel)" <peterz@infradead.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 5/5] pstore: inode: Use cleanup.h for struct pstore_private
Date: Sat, 2 Dec 2023 22:27:06 +0000 [thread overview]
Message-ID: <20231202222706.GT38156@ZenIV> (raw)
In-Reply-To: <20231202212217.243710-5-keescook@chromium.org>
On Sat, Dec 02, 2023 at 01:22:15PM -0800, Kees Cook wrote:
> static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos)
> {
> @@ -338,9 +339,8 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
> {
> struct dentry *dentry;
> struct inode *inode __free(iput) = NULL;
> - int rc = 0;
> char name[PSTORE_NAMELEN];
> - struct pstore_private *private, *pos;
> + struct pstore_private *private __free(pstore_private) = NULL, *pos;
> size_t size = record->size + record->ecc_notice_size;
>
> if (WARN_ON(!inode_is_locked(d_inode(root))))
> @@ -356,7 +356,6 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
> return -EEXIST;
> }
>
> - rc = -ENOMEM;
> inode = pstore_get_inode(root->d_sb);
> if (!inode)
> return -ENOMEM;
> @@ -373,7 +372,7 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
>
> dentry = d_alloc_name(root, name);
> if (!dentry)
> - goto fail_private;
> + return -ENOMEM;
>
> private->dentry = dentry;
> private->record = record;
> @@ -386,13 +385,9 @@ int pstore_mkfile(struct dentry *root, struct pstore_record *record)
>
> d_add(dentry, no_free_ptr(inode));
>
> - list_add(&private->list, &records_list);
> + list_add(&(no_free_ptr(private))->list, &records_list);
That's really brittle. It critically depends upon having no failure
exits past the assignment to ->i_private; once you've done that,
you have transferred the ownership of that thing to the inode
(look at your ->evict_inode()). But you can't say
inode->i_private = no_free_ptr(private);
since you are using private past that point.
next prev parent reply other threads:[~2023-12-02 22:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-02 21:22 [PATCH 0/5] pstore: Initial use of cleanup.h Kees Cook
2023-12-02 21:22 ` [PATCH 1/5] pstore: inode: Convert kfree() usage to __free(kfree) Kees Cook
2023-12-02 21:22 ` [PATCH 2/5] pstore: inode: Convert mutex usage to guard(mutex) Kees Cook
2023-12-05 7:01 ` Dave Chinner
2023-12-02 21:22 ` [PATCH 3/5] fs: Add DEFINE_FREE for struct inode Kees Cook
2023-12-02 21:28 ` Al Viro
2023-12-02 21:34 ` Kees Cook
2023-12-02 21:42 ` Al Viro
2023-12-02 21:45 ` Al Viro
2023-12-05 11:38 ` Christian Brauner
2023-12-02 21:22 ` [PATCH 4/5] pstore: inode: Use __free(iput) for inode allocations Kees Cook
2023-12-02 21:22 ` [PATCH 5/5] pstore: inode: Use cleanup.h for struct pstore_private Kees Cook
2023-12-02 22:27 ` Al Viro [this message]
2023-12-05 0:54 ` Kees Cook
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=20231202222706.GT38156@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=brauner@kernel.org \
--cc=gpiccoli@igalia.com \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=tony.luck@intel.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 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.