From: Siddh Raman Pant via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org> To: Eric Biggers <ebiggers@kernel.org>, Jonathan Corbet <corbet@lwn.net>, David Howells <dhowells@redhat.com>, Randy Dunlap <rdunlap@infradead.org>, Mauro Carvalho Chehab <mchehab@kernel.org>, Christophe JAILLET <christophe.jaillet@wanadoo.fr>, Eric Dumazet <edumazet@google.com> Cc: linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>, linux-kernel <linux-kernel@vger.kernel.org> Subject: [PATCH v2 2/2] kernel/watch_queue: NULL the dangling *pipe, and use it for clear check Date: Sat, 6 Aug 2022 13:13:42 +0530 [thread overview] Message-ID: <ef1791b7f1b5207d9ffae053d8641c9a634a25ea.1659771577.git.code@siddh.me> (raw) In-Reply-To: <cover.1659771577.git.code@siddh.me> NULL the dangling pipe reference while clearing watch_queue. If not done, a reference to a freed pipe remains in the watch_queue, as this function is called before freeing a pipe in free_pipe_info() (see line 834 of fs/pipe.c). The sole use of wqueue->defunct is for checking if the watch queue has been cleared, but wqueue->pipe is also NULLed while clearing. Thus, wqueue->defunct is superfluous, as wqueue->pipe can be checked for NULL. Hence, the former can be removed. Signed-off-by: Siddh Raman Pant <code@siddh.me> --- include/linux/watch_queue.h | 4 +--- kernel/watch_queue.c | 12 ++++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h index 7f8b1f15634b..d72ad82a4435 100644 --- a/include/linux/watch_queue.h +++ b/include/linux/watch_queue.h @@ -55,7 +55,7 @@ struct watch_filter { * * @rcu: RCU head * @filter: Filter to use on watches - * @pipe: The pipe we're using as a buffer + * @pipe: The pipe we're using as a buffer, NULL when queue is cleared/closed * @watches: Contributory watches * @notes: Preallocated notifications * @notes_bitmap: Allocation bitmap for notes @@ -63,7 +63,6 @@ struct watch_filter { * @lock: To serialize accesses and removes * @nr_notes: Number of notes * @nr_pages: Number of pages in notes[] - * @defunct: True when queues closed */ struct watch_queue { struct rcu_head rcu; @@ -76,7 +75,6 @@ struct watch_queue { spinlock_t lock; unsigned int nr_notes; unsigned int nr_pages; - bool defunct; }; /** diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c index a6f9bdd956c3..a70ddfd622ee 100644 --- a/kernel/watch_queue.c +++ b/kernel/watch_queue.c @@ -43,7 +43,7 @@ MODULE_LICENSE("GPL"); static inline bool lock_wqueue(struct watch_queue *wqueue) { spin_lock_bh(&wqueue->lock); - if (unlikely(wqueue->defunct)) { + if (unlikely(!wqueue->pipe)) { spin_unlock_bh(&wqueue->lock); return false; } @@ -105,9 +105,6 @@ static bool post_one_notification(struct watch_queue *wqueue, unsigned int head, tail, mask, note, offset, len; bool done = false; - if (!pipe) - return false; - spin_lock_irq(&pipe->rd_wait.lock); mask = pipe->ring_size - 1; @@ -603,8 +600,11 @@ void watch_queue_clear(struct watch_queue *wqueue) rcu_read_lock(); spin_lock_bh(&wqueue->lock); - /* Prevent new notifications from being stored. */ - wqueue->defunct = true; + /* + * This pipe will get freed by the caller free_pipe_info(). + * Removing this reference also prevents new notifications. + */ + wqueue->pipe = NULL; while (!hlist_empty(&wqueue->watches)) { watch = hlist_entry(wqueue->watches.first, struct watch, queue_node); -- 2.35.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
WARNING: multiple messages have this Message-ID (diff)
From: Siddh Raman Pant via Linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org> To: Jonathan Corbet <corbet@lwn.net>, David Howells <dhowells@redhat.com>, Randy Dunlap <rdunlap@infradead.org>, Mauro Carvalho Chehab <mchehab@kernel.org>, Christophe JAILLET <christophe.jaillet@wanadoo.fr>, Eric Dumazet <edumazet@google.com>, Eric Biggers <ebiggers@kernel.org> Cc: linux-kernel-mentees <linux-kernel-mentees@lists.linuxfoundation.org>, linux-kernel <linux-kernel@vger.kernel.org> Subject: [RESEND PATCH v2 2/2] kernel/watch_queue: NULL the dangling *pipe, and use it for clear check Date: Fri, 2 Sep 2022 01:36:56 +0530 [thread overview] Message-ID: <ef1791b7f1b5207d9ffae053d8641c9a634a25ea.1659771577.git.code@siddh.me> (raw) Message-ID: <20220901200656.89mWDLT7alrDE4aUUe1yLxnXS8jrXjAad0C8imXwXs0@z> (raw) In-Reply-To: <cover.1659771577.git.code@siddh.me> NULL the dangling pipe reference while clearing watch_queue. If not done, a reference to a freed pipe remains in the watch_queue, as this function is called before freeing a pipe in free_pipe_info() (see line 834 of fs/pipe.c). The sole use of wqueue->defunct is for checking if the watch queue has been cleared, but wqueue->pipe is also NULLed while clearing. Thus, wqueue->defunct is superfluous, as wqueue->pipe can be checked for NULL. Hence, the former can be removed. Signed-off-by: Siddh Raman Pant <code@siddh.me> --- include/linux/watch_queue.h | 4 +--- kernel/watch_queue.c | 12 ++++++------ 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h index 7f8b1f15634b..d72ad82a4435 100644 --- a/include/linux/watch_queue.h +++ b/include/linux/watch_queue.h @@ -55,7 +55,7 @@ struct watch_filter { * * @rcu: RCU head * @filter: Filter to use on watches - * @pipe: The pipe we're using as a buffer + * @pipe: The pipe we're using as a buffer, NULL when queue is cleared/closed * @watches: Contributory watches * @notes: Preallocated notifications * @notes_bitmap: Allocation bitmap for notes @@ -63,7 +63,6 @@ struct watch_filter { * @lock: To serialize accesses and removes * @nr_notes: Number of notes * @nr_pages: Number of pages in notes[] - * @defunct: True when queues closed */ struct watch_queue { struct rcu_head rcu; @@ -76,7 +75,6 @@ struct watch_queue { spinlock_t lock; unsigned int nr_notes; unsigned int nr_pages; - bool defunct; }; /** diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c index a6f9bdd956c3..a70ddfd622ee 100644 --- a/kernel/watch_queue.c +++ b/kernel/watch_queue.c @@ -43,7 +43,7 @@ MODULE_LICENSE("GPL"); static inline bool lock_wqueue(struct watch_queue *wqueue) { spin_lock_bh(&wqueue->lock); - if (unlikely(wqueue->defunct)) { + if (unlikely(!wqueue->pipe)) { spin_unlock_bh(&wqueue->lock); return false; } @@ -105,9 +105,6 @@ static bool post_one_notification(struct watch_queue *wqueue, unsigned int head, tail, mask, note, offset, len; bool done = false; - if (!pipe) - return false; - spin_lock_irq(&pipe->rd_wait.lock); mask = pipe->ring_size - 1; @@ -603,8 +600,11 @@ void watch_queue_clear(struct watch_queue *wqueue) rcu_read_lock(); spin_lock_bh(&wqueue->lock); - /* Prevent new notifications from being stored. */ - wqueue->defunct = true; + /* + * This pipe will get freed by the caller free_pipe_info(). + * Removing this reference also prevents new notifications. + */ + wqueue->pipe = NULL; while (!hlist_empty(&wqueue->watches)) { watch = hlist_entry(wqueue->watches.first, struct watch, queue_node); -- 2.35.1 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2022-08-06 7:44 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-08-06 7:43 [PATCH v2 0/2] watch_queue: Clean up some code Siddh Raman Pant via Linux-kernel-mentees 2022-08-06 7:43 ` [PATCH v2 1/2] include/linux/watch_queue: Improve documentation Siddh Raman Pant via Linux-kernel-mentees 2022-09-01 20:06 ` [RESEND PATCH " Siddh Raman Pant via Linux-kernel-mentees 2022-08-06 7:43 ` Siddh Raman Pant via Linux-kernel-mentees [this message] 2022-09-01 20:06 ` [RESEND PATCH v2 2/2] kernel/watch_queue: NULL the dangling *pipe, and use it for clear check Siddh Raman Pant via Linux-kernel-mentees 2022-08-20 11:28 ` [PATCH v2 0/2] watch_queue: Clean up some code Siddh Raman Pant via Linux-kernel-mentees 2022-09-01 20:06 ` [RESEND PATCH " Siddh Raman Pant via Linux-kernel-mentees
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=ef1791b7f1b5207d9ffae053d8641c9a634a25ea.1659771577.git.code@siddh.me \ --to=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=christophe.jaillet@wanadoo.fr \ --cc=code@siddh.me \ --cc=corbet@lwn.net \ --cc=dhowells@redhat.com \ --cc=ebiggers@kernel.org \ --cc=edumazet@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=rdunlap@infradead.org \ /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: linkBe 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; as well as URLs for NNTP newsgroup(s).