sched: sched_setattr/sched_getattr fixes and improvements
Subsystems (3)
cover sched: sched_setattr/sched_getattr fixes and improvements
Peter,
While looking at your sched_setattr()/sched_getatr() documentation, I noticed a bug in the handling of -EFBIG, and a couple of places in the code where the error handling logic could be streamlined. Patches follow.
Cheers,
Michael
Signature · 3 lines
Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
PATCH 1/3 sched: Make sched_setattr() correctly return -EFBIG
From: Michael Kerrisk <[email protected]>
The documented[1] behavior of sched_attr() in your proposed man page text is:
sched_attr::size must be set to the size of the structure, as in sizeof(struct sched_attr), if the provided structure is smaller than the kernel structure, any additional fields are assumed '0'. If the provided structure is larger than the kernel structure, the kernel verifies all additional fields are '0' if not the syscall will fail with -E2BIG.
As currently implemented, sched_copy_attr() returns -EFBIG for for this case, but the logic in sys_sched_setattr() converts that error to -EFAULT. This patch fixes the behavior.
[1] http://thread.gmane.org/gmane.linux.kernel/1615615/focus=1697760
--- kernel/sched/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/sched/core.c +3 -2
@@ -3650,8 +3650,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,if (!uattr || pid <0 || flags)return -EINVAL ; -if (sched_copy_attr (uattr, &attr)) -return -EFAULT ; + retval =sched_copy_attr (uattr, &attr); +if (retval) +return retval;rcu_read_lock (); retval = -ESRCH ;
Signature · 1 line
1.9.0
Re: [PATCH 1/3] sched: Make sched_setattr() correctly return -EFBIG
Peter,
In the title of this patch, EFBIG should have been E2BIG. Shall I resubmit a corrected version?
Cheers,
Michael
Quoted 39 lines · Michael Kerrisk (man-pages)
From: Michael Kerrisk <[email protected]>
The documented[1] behavior of
sched_attr()in your proposed man page text is:
sched_attr::sizemust be set to the size of the structure, as insizeof(struct sched_attr), if the provided structure is smaller than the kernel structure, any additional fields are assumed '0'. If the provided structure is larger than the kernel structure, the kernel verifies all additional fields are '0' if not the syscall will fail with-E2BIG.As currently implemented,
sched_copy_attr()returns-EFBIGfor for this case, but the logic insys_sched_setattr()converts that error to-EFAULT. This patch fixes the behavior.[1] http://thread.gmane.org/gmane.linux.kernel/1615615/focus=1697760
Signed-off-by Michael Kerrisk <[email protected]>--- kernel/sched/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)kernel/sched/core.c +3
-2@@ -3650,8 +3650,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,if (!uattr || pid <0 || flags)return -EINVAL ; -if (sched_copy_attr (uattr, &attr)) -return -EFAULT ; + retval =sched_copy_attr (uattr, &attr); +if (retval) +return retval;rcu_read_lock (); retval = -ESRCH ;
Signature · 7 lines
Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/ -- To unsubscribe from this list: send the line "unsubscribe linux-man" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Make sched_setattr() correctly return -EFBIG
In reply to Michael Kerrisk (man-pages)sched: Make sched_setattr() correctly return -EFBIG
Commit-ID: 51b2b2dc49a8a053de58b836be7efd2401a88699 Gitweb: http://git.kernel.org/tip/51b2b2dc49a8a053de58b836be7efd2401a88699 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:15 +0200 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 19 May 2014 21:47:33 +0900
sched: Make sched_setattr() correctly return -EFBIG
The documented[1] behavior of sched_attr() in your proposed man page text is:
sched_attr::size must be set to the size of the structure, as in sizeof(struct sched_attr), if the provided structure is smaller than the kernel structure, any additional fields are assumed '0'. If the provided structure is larger than the kernel structure, the kernel verifies all additional fields are '0' if not the syscall will fail with -E2BIG.
As currently implemented, sched_copy_attr() returns -EFBIG for for this case, but the logic in sys_sched_setattr() converts that error to -EFAULT. This patch fixes the behavior.
[1] http://thread.gmane.org/gmane.linux.kernel/1615615/focus=1697760
--- kernel/sched/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/sched/core.c +3 -2
@@ -3658,8 +3658,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,if (!uattr || pid <0 || flags)return -EINVAL ; -if (sched_copy_attr (uattr, &attr)) -return -EFAULT ; + retval =sched_copy_attr (uattr, &attr); +if (retval) +return retval;rcu_read_lock (); retval = -ESRCH ;
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Make sched_setattr() correctly return -EFBIG
In reply to Michael Kerrisk (man-pages)sched: Make sched_setattr() correctly return -EFBIG
Commit-ID: 143cf23df25b7082cd706c3c53188e741e7881c3 Gitweb: http://git.kernel.org/tip/143cf23df25b7082cd706c3c53188e741e7881c3 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:15 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 22 May 2014 10:21:25 +0200
sched: Make sched_setattr() correctly return -EFBIG
The documented[1] behavior of sched_attr() in the proposed man page text is:
sched_attr::size must be set to the size of the structure, as in sizeof(struct sched_attr), if the provided structure is smaller than the kernel structure, any additional fields are assumed '0'. If the provided structure is larger than the kernel structure, the kernel verifies all additional fields are '0' if not the syscall will fail with -E2BIG.
As currently implemented, sched_copy_attr() returns -EFBIG for for this case, but the logic in sys_sched_setattr() converts that error to -EFAULT. This patch fixes the behavior.
[1] http://thread.gmane.org/gmane.linux.kernel/1615615/focus=1697760
--- kernel/sched/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
kernel/sched/core.c +3 -2
@@ -3658,8 +3658,9 @@ SYSCALL_DEFINE3(sched_setattr, pid_t, pid, struct sched_attr __user *, uattr,if (!uattr || pid <0 || flags)return -EINVAL ; -if (sched_copy_attr (uattr, &attr)) -return -EFAULT ; + retval =sched_copy_attr (uattr, &attr); +if (retval) +return retval;rcu_read_lock (); retval = -ESRCH ;
Re: [PATCH 2/3] sched: Simplify return logic in sched_copy_attr()
In reply to Michael Kerrisk (man-pages)From: Michael Kerrisk <[email protected]>
The logic in this function is a little contorted:
* Rather than having chained gotos for the -EFBIG case, just return -EFBIG directly.
* Now, the label 'out' is no longer needed, and 'ret' must be zero zero by the time we fall through to this point, so just return 0.
--- kernel/sched/core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/sched/core.c +2 -4
@@ -3596,13 +3596,11 @@ static int sched_copy_attr(struct sched_attr __user *uattr,*/ attr->sched_nice =clamp (attr->sched_nice,MIN_NICE ,MAX_NICE ); -out: -return ret; +return 0 ; err_size:put_user (sizeof (*attr), &uattr->size); - ret = -E2BIG ; - goto out; +return -E2BIG ; }/**
Signature · 1 line
1.9.0
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Simplify return logic in sched_copy_attr( )
sched: Simplify return logic in sched_copy_attr()
Commit-ID: 814927325e815b667a133d9dcdf2286f88b78328 Gitweb: http://git.kernel.org/tip/814927325e815b667a133d9dcdf2286f88b78328 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:28 +0200 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 19 May 2014 22:02:39 +0900
sched: Simplify return logic in sched_copy_attr()
The logic in this function is a little contorted:
* Rather than having chained gotos for the -EFBIG case, just return -EFBIG directly.
* Now, the label 'out' is no longer needed, and 'ret' must be zero zero by the time we fall through to this point, so just return 0.
--- kernel/sched/core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/sched/core.c +2 -4
@@ -3657,13 +3657,11 @@ static int sched_copy_attr(struct sched_attr __user *uattr,*/ attr->sched_nice =clamp (attr->sched_nice,MIN_NICE ,MAX_NICE ); -out: -return ret; +return 0 ; err_size:put_user (sizeof (*attr), &uattr->size); - ret = -E2BIG ; - goto out; +return -E2BIG ; }/**
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Simplify return logic in sched_copy_attr( )
In reply to Michael Kerrisk (man-pages)sched: Simplify return logic in sched_copy_attr()
Commit-ID: e78c7bca56dab5ce4f22694f99115ec07e4935f6 Gitweb: http://git.kernel.org/tip/e78c7bca56dab5ce4f22694f99115ec07e4935f6 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:28 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 22 May 2014 11:16:26 +0200
sched: Simplify return logic in sched_copy_attr()
The logic in this function is a little contorted, clean it up:
* Rather than having chained gotos for the -EFBIG case, just return -EFBIG directly.
* Now, the label 'out' is no longer needed, and 'ret' must be zero zero by the time we fall through to this point, so just return 0.
--- kernel/sched/core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/sched/core.c +2 -4
@@ -3657,13 +3657,11 @@ static int sched_copy_attr(struct sched_attr __user *uattr,*/ attr->sched_nice =clamp (attr->sched_nice,MIN_NICE ,MAX_NICE ); -out: -return ret; +return 0 ; err_size:put_user (sizeof (*attr), &uattr->size); - ret = -E2BIG ; - goto out; +return -E2BIG ; }/**
PATCH 3/3 sched: Simplify return logic in sched_read_attr()
In reply to Michael Kerrisk (man-pages)From: Michael Kerrisk <[email protected]>
Gotos are chained pointlessly here, and the 'out' label can be dispensed with.
--- kernel/sched/core.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
kernel/sched/core.c +2 -7
@@ -3759,7 +3759,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,for (; addr < end; addr++) {if (*addr) - goto err_size; +return -EFBIG ; } attr->size = usize;
@@ -3769,12 +3769,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,if (ret)return -EFAULT ; -out: -return ret; - -err_size: - ret = -E2BIG ; - goto out; +return 0 ; }/**
Signature · 1 line
1.9.0
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Simplify return logic in sched_read_attr( )
sched: Simplify return logic in sched_read_attr()
Commit-ID: 55854e657f9890a1f76105c698d1d52cf6c43661 Gitweb: http://git.kernel.org/tip/55854e657f9890a1f76105c698d1d52cf6c43661 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:33 +0200 Committer: Thomas Gleixner <[email protected]> CommitDate: Mon, 19 May 2014 22:02:40 +0900
sched: Simplify return logic in sched_read_attr()
Gotos are chained pointlessly here, and the 'out' label can be dispensed with.
--- kernel/sched/core.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
kernel/sched/core.c +2 -7
@@ -3821,7 +3821,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,for (; addr < end; addr++) {if (*addr) - goto err_size; +return -EFBIG ; } attr->size = usize;
@@ -3831,12 +3831,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,if (ret)return -EFAULT ; -out: -return ret; - -err_size: - ret = -E2BIG ; - goto out; +return 0 ; }/**
tip-bot for Michael Kerrisk
merge bot[tip:sched/core] sched: Simplify return logic in sched_read_attr( )
In reply to Michael Kerrisk (man-pages)sched: Simplify return logic in sched_read_attr()
Commit-ID: 22400674945c562cf3c176d6c4178cd545e2dec9 Gitweb: http://git.kernel.org/tip/22400674945c562cf3c176d6c4178cd545e2dec9 Author: Michael Kerrisk <[email protected]> AuthorDate: Fri, 9 May 2014 16:54:33 +0200 Committer: Ingo Molnar <[email protected]> CommitDate: Thu, 22 May 2014 11:16:27 +0200
sched: Simplify return logic in sched_read_attr()
Gotos are chained pointlessly here, and the 'out' label can be dispensed with.
--- kernel/sched/core.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
kernel/sched/core.c +2 -7
@@ -3821,7 +3821,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,for (; addr < end; addr++) {if (*addr) - goto err_size; +return -EFBIG ; } attr->size = usize;
@@ -3831,12 +3831,7 @@ static int sched_read_attr(struct sched_attr __user *uattr,if (ret)return -EFAULT ; -out: -return ret; - -err_size: - ret = -E2BIG ; - goto out; +return 0 ; }/**
Peter Zijlstra
maintainerQuoted 6 lines · Michael Kerrisk (man-pages)
Peter,
While looking at your sched_setattr()/sched_getatr() documentation, I noticed a bug in the handling of
-EFBIG, and a couple of places in the code where the error handling logic could be streamlined. Patches follow.
Thanks!
Quoted 9 lines · Peter Zijlstra
Quoted 6 lines · Michael Kerrisk (man-pages)
Peter,
While looking at your sched_setattr()/sched_getatr() documentation, I noticed a bug in the handling of
-EFBIG, and a couple of places in the code where the error handling logic could be streamlined. Patches follow.Thanks!
Hi Peter,
I was a little unclear on what "Thanks" meant... Does it mean you plan to apply all 3 patches in your tree to go upstream?
Thanks,
Michael
Signature · 3 lines
Michael Kerrisk Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ Linux/UNIX System Programming Training: http://man7.org/training/
Peter Zijlstra
maintainerQuoted 9 lines · Michael Kerrisk (man-pages)
Quoted 8 lines · Peter Zijlstra
Quoted 6 lines · Michael Kerrisk (man-pages)
Peter,
While looking at your sched_setattr()/sched_getatr() documentation, I noticed a bug in the handling of
-EFBIG, and a couple of places in the code where the error handling logic could be streamlined. Patches follow.
Quoted 6 lines
Quoted 1 line
Thanks!
Hi Peter,
I was a little unclear on what "Thanks" meant... Does it mean you plan to apply all 3 patches in your tree to go upstream?
Yeah, I queued them. They should appear in tip sometime this week or so.