* [PATCH] librt: fix NULL pointer dereference (bug 28213)
@ 2021-08-09 12:25 Никита Попов via Libc-alpha
2021-08-09 13:21 ` Siddhesh Poyarekar
0 siblings, 1 reply; 7+ messages in thread
From: Никита Попов via Libc-alpha @ 2021-08-09 12:25 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-librt-fix-NULL-pointer-dereference-bug-28213.patch --]
[-- Type: text/x-patch, Size: 1557 bytes --]
From c69f990e356dd8e756b0025e026d59db5af6e059 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npv1310@gmail.com>
Date: Mon, 9 Aug 2021 17:15:52 +0500
Subject: [PATCH] librt: fix NULL pointer dereference (bug 28213)
To: libc-alpha@sourceware.org
Helper thread frees copied attribute on NOTIFY_REMOVED message received from the OS kernel. Unfortunately, it fails to check whether copied attribute actually exists (data.attr != NULL). This worked earlier because free() checks passed pointer before actually attempting to release corresponding memory. But __pthread_attr_destroy assumes pointer is not NULL. So passing NULL pointer to __pthread_attr_destroy will result in segmentation fault. This scenario is possible if notification->sigev_notify_attributes == NULL (which means default thread attributes should be used).
---
sysdeps/unix/sysv/linux/mq_notify.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/unix/sysv/linux/mq_notify.c b/sysdeps/unix/sysv/linux/mq_notify.c
index 9799dcdaa4..eccae2e4c6 100644
--- a/sysdeps/unix/sysv/linux/mq_notify.c
+++ b/sysdeps/unix/sysv/linux/mq_notify.c
@@ -131,7 +131,7 @@ helper_thread (void *arg)
to wait until it is done with it. */
(void) __pthread_barrier_wait (¬ify_barrier);
}
- else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED)
+ else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED && data.attr != NULL)
{
/* The only state we keep is the copy of the thread attributes. */
__pthread_attr_destroy (data.attr);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 12:25 [PATCH] librt: fix NULL pointer dereference (bug 28213) Никита Попов via Libc-alpha
@ 2021-08-09 13:21 ` Siddhesh Poyarekar
2021-08-09 13:27 ` Florian Weimer via Libc-alpha
2021-08-09 13:32 ` Siddhesh Poyarekar
0 siblings, 2 replies; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-09 13:21 UTC (permalink / raw)
To: Никита Попов,
libc-alpha
On 8/9/21 5:55 PM, Никита Попов via Libc-alpha wrote:
> Helper thread frees copied attribute on NOTIFY_REMOVED message received from the OS kernel. Unfortunately, it fails to check whether copied attribute actually exists (data.attr != NULL). This worked earlier because free() checks passed pointer before actually attempting to release corresponding memory. But __pthread_attr_destroy assumes pointer is not NULL. So passing NULL pointer to __pthread_attr_destroy will result in segmentation fault. This scenario is possible if notification->sigev_notify_attributes == NULL (which means default thread attributes should be used).
Thank you, the fix looks good to me. Do you have a test case to go with it?
Thanks,
Siddhesh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 13:21 ` Siddhesh Poyarekar
@ 2021-08-09 13:27 ` Florian Weimer via Libc-alpha
2021-08-09 13:32 ` Siddhesh Poyarekar
1 sibling, 0 replies; 7+ messages in thread
From: Florian Weimer via Libc-alpha @ 2021-08-09 13:27 UTC (permalink / raw)
To: Siddhesh Poyarekar
Cc: libc-alpha,
Никита Попов
* Siddhesh Poyarekar:
> On 8/9/21 5:55 PM, Никита Попов via Libc-alpha wrote:
>> Helper thread frees copied attribute on NOTIFY_REMOVED message received from the OS kernel. Unfortunately, it fails to check whether copied attribute actually exists (data.attr != NULL). This worked earlier because free() checks passed pointer before actually attempting to release corresponding memory. But __pthread_attr_destroy assumes pointer is not NULL. So passing NULL pointer to __pthread_attr_destroy will result in segmentation fault. This scenario is possible if notification->sigev_notify_attributes == NULL (which means default thread attributes should be used).
>
> Thank you, the fix looks good to me. Do you have a test case to go with it?
Siddhesh, if you are going to push this, please line-wrap the commit
message before doing so.
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 13:21 ` Siddhesh Poyarekar
2021-08-09 13:27 ` Florian Weimer via Libc-alpha
@ 2021-08-09 13:32 ` Siddhesh Poyarekar
2021-08-09 13:45 ` Никита Попов via Libc-alpha
1 sibling, 1 reply; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-09 13:32 UTC (permalink / raw)
To: Никита Попов,
libc-alpha
On 8/9/21 6:51 PM, Siddhesh Poyarekar wrote:
> On 8/9/21 5:55 PM, Никита Попов via Libc-alpha wrote:
>> Helper thread frees copied attribute on NOTIFY_REMOVED message
>> received from the OS kernel. Unfortunately, it fails to check whether
>> copied attribute actually exists (data.attr != NULL). This worked
>> earlier because free() checks passed pointer before actually
>> attempting to release corresponding memory. But __pthread_attr_destroy
>> assumes pointer is not NULL. So passing NULL pointer to
>> __pthread_attr_destroy will result in segmentation fault. This
>> scenario is possible if notification->sigev_notify_attributes == NULL
>> (which means default thread attributes should be used).
>
> Thank you, the fix looks good to me. Do you have a test case to go with
> it?
Also, I don't know if you have an FSF copyright assignment, but it's no
longer necessary. Please confirm that you're the original author and
are authorized to contribute this patch by adding a DCO, i.e. add a
Signed-off-by to indicate that. See also:
https://developercertificate.org/
Thanks,
Siddhesh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 13:32 ` Siddhesh Poyarekar
@ 2021-08-09 13:45 ` Никита Попов via Libc-alpha
2021-08-09 14:35 ` Siddhesh Poyarekar
2021-08-09 14:50 ` Siddhesh Poyarekar
0 siblings, 2 replies; 7+ messages in thread
From: Никита Попов via Libc-alpha @ 2021-08-09 13:45 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
Thanks for the feedback. Yes, I confirm that I'm the original author
of this patch. Here is the adjusted version. If necessary, I can write
proof-of-concept and attach it here.
пн, 9 авг. 2021 г. в 18:32, Siddhesh Poyarekar <siddhesh@gotplt.org>:
>
> On 8/9/21 6:51 PM, Siddhesh Poyarekar wrote:
> > On 8/9/21 5:55 PM, Никита Попов via Libc-alpha wrote:
> >> Helper thread frees copied attribute on NOTIFY_REMOVED message
> >> received from the OS kernel. Unfortunately, it fails to check whether
> >> copied attribute actually exists (data.attr != NULL). This worked
> >> earlier because free() checks passed pointer before actually
> >> attempting to release corresponding memory. But __pthread_attr_destroy
> >> assumes pointer is not NULL. So passing NULL pointer to
> >> __pthread_attr_destroy will result in segmentation fault. This
> >> scenario is possible if notification->sigev_notify_attributes == NULL
> >> (which means default thread attributes should be used).
> >
> > Thank you, the fix looks good to me. Do you have a test case to go with
> > it?
>
> Also, I don't know if you have an FSF copyright assignment, but it's no
> longer necessary. Please confirm that you're the original author and
> are authorized to contribute this patch by adding a DCO, i.e. add a
> Signed-off-by to indicate that. See also:
>
> https://developercertificate.org/
>
> Thanks,
> Siddhesh
[-- Attachment #2: 0001-librt-fix-NULL-pointer-dereference-bug-28213.patch --]
[-- Type: text/x-patch, Size: 1606 bytes --]
From 3ccf46039ce31c60436776446202f9905be6fea6 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npv1310@gmail.com>
Date: Mon, 9 Aug 2021 18:39:40 +0500
Subject: [PATCH] librt: fix NULL pointer dereference (bug 28213)
To: libc-alpha@sourceware.org
Helper thread frees copied attribute on NOTIFY_REMOVED message received from the OS kernel.
Unfortunately, it fails to check whether copied attribute actually exists (data.attr != NULL).
This worked earlier because free() checks passed pointer before actually attempting to release corresponding memory.
But __pthread_attr_destroy assumes pointer is not NULL.
So passing NULL pointer to __pthread_attr_destroy will result in segmentation fault.
This scenario is possible if notification->sigev_notify_attributes == NULL
(which means default thread attributes should be used).
Signed-off-by: Nikita Popov <npv1310@gmail.com>
---
sysdeps/unix/sysv/linux/mq_notify.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sysdeps/unix/sysv/linux/mq_notify.c b/sysdeps/unix/sysv/linux/mq_notify.c
index 9799dcdaa4..eccae2e4c6 100644
--- a/sysdeps/unix/sysv/linux/mq_notify.c
+++ b/sysdeps/unix/sysv/linux/mq_notify.c
@@ -131,7 +131,7 @@ helper_thread (void *arg)
to wait until it is done with it. */
(void) __pthread_barrier_wait (¬ify_barrier);
}
- else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED)
+ else if (data.raw[NOTIFY_COOKIE_LEN - 1] == NOTIFY_REMOVED && data.attr != NULL)
{
/* The only state we keep is the copy of the thread attributes. */
__pthread_attr_destroy (data.attr);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 13:45 ` Никита Попов via Libc-alpha
@ 2021-08-09 14:35 ` Siddhesh Poyarekar
2021-08-09 14:50 ` Siddhesh Poyarekar
1 sibling, 0 replies; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-09 14:35 UTC (permalink / raw)
To: Никита Попов,
libc-alpha
On 8/9/21 7:15 PM, Никита Попов via Libc-alpha wrote:
> Thanks for the feedback. Yes, I confirm that I'm the original author
> of this patch. Here is the adjusted version. If necessary, I can write
> proof-of-concept and attach it here.
If the PoC is deterministic, it would be useful as a test case[1] to
protect from future regressions.
Thanks,
Siddhesh
[1] https://sourceware.org/glibc/wiki/Testing/Testsuite
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] librt: fix NULL pointer dereference (bug 28213)
2021-08-09 13:45 ` Никита Попов via Libc-alpha
2021-08-09 14:35 ` Siddhesh Poyarekar
@ 2021-08-09 14:50 ` Siddhesh Poyarekar
1 sibling, 0 replies; 7+ messages in thread
From: Siddhesh Poyarekar @ 2021-08-09 14:50 UTC (permalink / raw)
To: Никита Попов,
libc-alpha
On 8/9/21 7:15 PM, Никита Попов via Libc-alpha wrote:
> Thanks for the feedback. Yes, I confirm that I'm the original author
> of this patch. Here is the adjusted version. If necessary, I can write
> proof-of-concept and attach it here.
I have pushed your patch now, thanks again. We can push the test as a
separate patch if it is deterministic.
Thanks,
Siddhesh
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-09 14:51 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-09 12:25 [PATCH] librt: fix NULL pointer dereference (bug 28213) Никита Попов via Libc-alpha
2021-08-09 13:21 ` Siddhesh Poyarekar
2021-08-09 13:27 ` Florian Weimer via Libc-alpha
2021-08-09 13:32 ` Siddhesh Poyarekar
2021-08-09 13:45 ` Никита Попов via Libc-alpha
2021-08-09 14:35 ` Siddhesh Poyarekar
2021-08-09 14:50 ` Siddhesh Poyarekar
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 read-only IMAP folder(s) and NNTP newsgroup(s).