From patchwork Tue Nov 18 20:52:13 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland McGrath X-Patchwork-Id: 3796 Received: (qmail 28917 invoked by alias); 18 Nov 2014 20:52:17 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 28893 invoked by uid 89); 18 Nov 2014 20:52:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00 autolearn=ham version=3.3.2 X-HELO: topped-with-meat.com MIME-Version: 1.0 From: Roland McGrath To: "GNU C. Library" Subject: [PATCH roland/nptl] NPTL: Don't (re)validate sched_priority in pthread_create. Message-Id: <20141118205213.90CF92C3B21@topped-with-meat.com> Date: Tue, 18 Nov 2014 12:52:13 -0800 (PST) X-CMAE-Score: 0 X-CMAE-Analysis: v=2.1 cv=SvUDtp+0 c=1 sm=1 tr=0 a=WkljmVdYkabdwxfqvArNOQ==:117 a=14OXPxybAAAA:8 a=kj9zAlcOel0A:10 a=hOe2yjtxAAAA:8 a=h_25X6ZIMyge40B-PgwA:9 a=CjuIK1q_8ugA:10 Now that pthread_attr_setschedparam validates the value, it should no longer be possible to see an invalid value in pthread_create. I've verified that in Linux the min/max values depend only on the policy selection and not on any dynamic factors that might have changed between pthread_attr_setschedparam and pthread_create. The one wrinkle is if the user called pthread_attr_setschedparam before pthread_attr_setschedpolicy and the sched_priority value was valid for the old policy but invalid for the new one. POSIX does not seem to provide for pthread_attr_setschedpolicy being able to return an error in this case, or for it to do something else like reset the sched_param bits to defaults on pthread_attr_setschedpolicy if the old bits don't make sense for the new policy. So in this situation, pthread_create failing with EINVAL appears to be the best we can do--and that's what we do now. But removing this check in pthread_create doesn't change that behavior observably: the thread will be started up and then create_thread will cancel it when the sched_setscheduler syscall fails, propagating the error code (presumably still EINVAL) back to pthread_create. So this user misuse of the API is detected with significantly more overhead, but I don't think that matters. Tested x86_64-linux-gnu. If nobody objects soon, I'll commit this. Thanks, Roland * nptl/pthread_create.c (__pthread_create_2_1): Don't try to validate the sched_priority value here. It was already checked when the user called pthread_attr_setschedparam. --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -605,6 +605,7 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg) } if (iattr->flags & ATTR_FLAG_SCHED_SET) + /* The values were validated in pthread_attr_setschedparam. */ memcpy (&pd->schedparam, &iattr->schedparam, sizeof (struct sched_param)); else if ((pd->flags & ATTR_FLAG_SCHED_SET) == 0) @@ -612,26 +613,6 @@ __pthread_create_2_1 (newthread, attr, start_routine, arg) INTERNAL_SYSCALL (sched_getparam, scerr, 2, 0, &pd->schedparam); pd->flags |= ATTR_FLAG_SCHED_SET; } - - /* Check for valid priorities. */ - int minprio = INTERNAL_SYSCALL (sched_get_priority_min, scerr, 1, - iattr->schedpolicy); - int maxprio = INTERNAL_SYSCALL (sched_get_priority_max, scerr, 1, - iattr->schedpolicy); - if (pd->schedparam.sched_priority < minprio - || pd->schedparam.sched_priority > maxprio) - { - /* Perhaps a thread wants to change the IDs and if waiting - for this stillborn thread. */ - if (__builtin_expect (atomic_exchange_acq (&pd->setxid_futex, 0) - == -2, 0)) - lll_futex_wake (&pd->setxid_futex, 1, LLL_PRIVATE); - - __deallocate_stack (pd); - - retval = EINVAL; - goto out; - } } /* Pass the descriptor to the caller. */