git: aadb4a1b3fd1 - main - pthread_attr_get_np(): Use malloc(), report ENOMEM, don't tamper on error

The branch main has been updated by olce:

URL: https://cgit.FreeBSD.org/src/commit/?id=aadb4a1b3fd185d547087f6eafca6ce0b4df3291

commit aadb4a1b3fd185d547087f6eafca6ce0b4df3291
Author:     Olivier Certner <olce@FreeBSD.org>
AuthorDate: 2024-01-04 17:45:52 +0000
Commit:     Olivier Certner <olce@FreeBSD.org>
CommitDate: 2024-01-10 17:50:19 +0000

    pthread_attr_get_np(): Use malloc(), report ENOMEM, don't tamper on error
    
    Similarly as in the previous commit, using calloc() instead of malloc()
    is useless here in the regular case since the subsequent call to
    cpuset_getaffinify() is going to completely fill the allocated memory.
    
    However, there is an additional complication.  This function tries to
    allocate memory to hold the cpuset if it previously wasn't, and does so
    before the thread lock is acquired, which can fail on a bad thread ID.
    In this case, it is necessary to deallocate the memory allocated in this
    function so that the attributes object appears unmodified to the caller
    when an error is returned.  Without this, a subsequent call to
    pthread_attr_getaffinity_np() would expose uninitialized memory (not
    a security problem per se, since it comes from the same process) instead
    of returning a full mask as it would before the failing call to
    pthread_attr_get_np().  So the caller would be able to notice a change
    in the state of the attributes object even if pthread_attr_get_np()
    reported failure, which would be quite surprising.  A similar problem
    that could occur on failure of cpuset_setaffinity() has been fixed.
    
    Finally, we shall always report memory allocation failure.  This already
    goes for pthread_attr_init(), so, if for nothing else, just be
    consistent.
    
    Reviewed by:            emaste, kib
    Approved by:            emaste (mentor)
    MFC after:              2 weeks
    Sponsored by:           The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D43329
---
 lib/libthr/thread/thr_attr.c    | 45 ++++++++++++++++++++++++-----------------
 lib/libthr/thread/thr_private.h |  2 --
 2 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/lib/libthr/thread/thr_attr.c b/lib/libthr/thread/thr_attr.c
index 561037fa5eb4..3c03225c33cb 100644
--- a/lib/libthr/thread/thr_attr.c
+++ b/lib/libthr/thread/thr_attr.c
@@ -129,8 +129,9 @@ __weak_reference(_thr_attr_get_np, _pthread_attr_get_np);
 int
 _thr_attr_get_np(pthread_t pthread, pthread_attr_t *dstattr)
 {
-	struct pthread_attr attr, *dst;
+	struct pthread_attr *dst;
 	struct pthread *curthread;
+	cpuset_t *cpuset;
 	size_t kern_size;
 	int error;
 
@@ -138,35 +139,43 @@ _thr_attr_get_np(pthread_t pthread, pthread_attr_t *dstattr)
 		return (EINVAL);
 
 	kern_size = _get_kern_cpuset_size();
-
 	if (dst->cpuset == NULL) {
-		dst->cpuset = calloc(1, kern_size);
-		dst->cpusetsize = kern_size;
-	}
+		if ((cpuset = malloc(kern_size)) == NULL)
+			return (ENOMEM);
+	} else
+		cpuset = dst->cpuset;
 
 	curthread = _get_curthread();
 	/* Arg 0 is to include dead threads. */
 	if ((error = _thr_find_thread(curthread, pthread, 0)) != 0)
-		return (error);
-
-	attr = pthread->attr;
-	if ((pthread->flags & THR_FLAGS_DETACHED) != 0)
-		attr.flags |= PTHREAD_DETACHED;
+		goto free_and_exit;
 
 	error = cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, TID(pthread),
-	    dst->cpusetsize, dst->cpuset);
-	if (error == -1)
+	    kern_size, cpuset);
+	if (error == -1) {
+		THR_THREAD_UNLOCK(curthread, pthread);
 		error = errno;
+		goto free_and_exit;
+	}
 
-	THR_THREAD_UNLOCK(curthread, pthread);
+	/*
+	 * From this point on, we can't fail, so we can start modifying 'dst'.
+	 */
 
-	if (error == 0)
-		memcpy(&dst->pthread_attr_start_copy,
-		    &attr.pthread_attr_start_copy,
-		    offsetof(struct pthread_attr, pthread_attr_end_copy) -
-		    offsetof(struct pthread_attr, pthread_attr_start_copy));
+	*dst = pthread->attr;
+	if ((pthread->flags & THR_FLAGS_DETACHED) != 0)
+		dst->flags |= PTHREAD_DETACHED;
 
+	THR_THREAD_UNLOCK(curthread, pthread);
+
+	dst->cpuset = cpuset;
+	dst->cpusetsize = kern_size;
 	return (0);
+
+free_and_exit:
+	if (dst->cpuset == NULL)
+		free(cpuset);
+	return (error);
 }
 
 __weak_reference(_thr_attr_getdetachstate, pthread_attr_getdetachstate);
diff --git a/lib/libthr/thread/thr_private.h b/lib/libthr/thread/thr_private.h
index 3475029f8996..3fc4d02c611e 100644
--- a/lib/libthr/thread/thr_private.h
+++ b/lib/libthr/thread/thr_private.h
@@ -261,7 +261,6 @@ struct pthread_atfork {
 };
 
 struct pthread_attr {
-#define pthread_attr_start_copy	sched_policy
 	int	sched_policy;
 	int	sched_inherit;
 	int	prio;
@@ -271,7 +270,6 @@ struct pthread_attr {
 	void	*stackaddr_attr;
 	size_t	stacksize_attr;
 	size_t	guardsize_attr;
-#define pthread_attr_end_copy	cpuset
 	cpuset_t	*cpuset;
 	size_t	cpusetsize;
 };