I cut the RISC-V stuff, but I intend to reply to it later. As you
said, it's just a different topic.
quoted
quoted
However, I did see a lot of similar bugs now that you point me to it:
$ grep -r \\\<CONFIG obj-tmp/usr/include/
obj-tmp/usr/include/asm-generic/fcntl.h:#ifndef CONFIG_64BIT
obj-tmp/usr/include/asm-generic/mman-common.h:#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
obj-tmp/usr/include/asm-generic/unistd.h:#ifdef CONFIG_MMU
obj-tmp/usr/include/asm-generic/unistd.h:#endif /* CONFIG_MMU */
obj-tmp/usr/include/linux/atmdev.h:#ifdef CONFIG_COMPAT
obj-tmp/usr/include/linux/elfcore.h:#ifdef CONFIG_BINFMT_ELF_FDPIC
obj-tmp/usr/include/linux/eventpoll.h:#ifdef CONFIG_PM_SLEEP
obj-tmp/usr/include/linux/fb.h:#ifdef CONFIG_FB_BACKLIGHT
obj-tmp/usr/include/linux/flat.h:#ifdef CONFIG_BINFMT_SHARED_FLAT
obj-tmp/usr/include/linux/hw_breakpoint.h:#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
obj-tmp/usr/include/linux/pktcdvd.h:#if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
obj-tmp/usr/include/linux/raw.h:#define MAX_RAW_MINORS CONFIG_MAX_RAW_DEVS
obj-tmp/usr/include/asm/ptrace.h:#ifdef CONFIG_CPU_ENDIAN_BE8
These all have the same problem, and we should fix them, as well as
(probably) adding an automated check to scripts/headers_install.sh.
Well, I was going to go fix them all and ran a very similar grep, but
I think I got a lot of false-positives. If I understand correctly,
it's allowed to have CONFIG_* when guarded by __KERNEL__ in
user-visible headers?
That is right.
It turns out there was actually a header checking script
(scripts/headers_check.pl), and it already had a check for this. The
check was just disabled because there was "too much noise". Rather
than putting it in headers_install I've just fixed that script. I'm
definately lacking in perl powers, so I have no idea if what I've done
is sane. Specifically: there's a global variable and a line over 80
characters, but since there's a bunch of other violations I figure
it's fine.
quoted
Now that I've written that, I realize it'd be pretty easy to just use
cpp to drop everything inside __KERNEL__ and then look for CONFIG_*.
The lines quoted above are from the output of 'make headers_install',
which already drops everything inside of __KERNEL__. A lot of them
probably just need to add that #ifdef, or move the portion of the
header file to the normal (non-uabi) file.
quoted
If you want, I can try to do that, fix what triggers the check, and
re-submit everything together?
That would be great, yes.
OK. I think this has turned into more of a RFC than a PATCH,
though... I've just #ifdef'd things for now to reduce the diff size,
though I think it might be cleaner to move some of them to the
non-user headers (ep_take_care_of_epollwakeup(), USE_WCACHING,
MAX_RAW_MINORS).
I'm pretty far out of my depth here, so these should all be carefully
considered, but there's a few that scare me more ("struct
elf_prstatus", "enum by_type_idx", AT_VECTOR_SIZE_ARCH). I think
there's only one actual bug here (MAP_UNINITIALIZED), the rest just
quiet the checking script. Each patch has my rationale for what I
did.
Since this touches a whole lot of stuff, I've added a whole bunch of
CCs.
When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops. Since there's not compat
layer for these, this causes fcntl to bail out.
It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/fcntl.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Nothing else in the kernel defines this, and this header is visible to
userspace. Rather than hiding it in an #ifdef, I think it's sane to
just make this visible to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/fb.h | 3 ---
1 file changed, 3 deletions(-)
I recently got bit by a CONFIG_ in userspace bug. This has apparently
happened before, but the check got disabled for triggering too much.
In order to reduce false positives, I added some hueristics to avoid
detecting comments.
Since these tests all pass, I've now re-enabled them.
Signed-off-by: Palmer Dabbelt <redacted>
Reviewed-by: Andrew Waterman <waterman-aFE07iDfcCIb0cFwG/AQJIdd74u8MsAO@public.gmane.org>
Reviewed-by: Albert Ou <aou-aFE07iDfcCIb0cFwG/AQJIdd74u8MsAO@public.gmane.org>
---
scripts/headers_check.pl | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
@@ -40,7 +40,7 @@ foreach my $file (@files) {&check_asm_types();&check_sizetypes();&check_declarations();-#Droppedfornow.Toomuchnoise&check_config();+&check_config();}close$fh;}
@@ -76,9 +76,24 @@ sub check_declarations}}+my$check_config_in_multiline_comment=0;subcheck_config{-if($line=~m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/){+my$nocomments=$line;+$nocomments=~s/\/\*.*\*\///;#RemoveANSI-stylecomments(/* to */)+$nocomments=~s/\/\/.*//;#RemoveC99-stylecomments(//toEOL)++#Checktoseeifwe'rewithinamultilinecomment,andifso+#justremovethewholeline.Itriedmatchingon'^ * ',but+#there's one header that doesn'tprependmulti-linecomments+#with*sothatwon'twork.+if($nocomments=~m/\/\*/){$check_config_in_multiline_comment=1;}+if($nocomments=~m/\*\//){$check_config_in_multiline_comment=0;}+if($check_config_in_multiline_comment==1){$nocomments=""}++#ChecktoseeifthereissomethingthatlookslikeCONFIG_+#insideauserspace-accessibleheaderfileandifso,printthatout.+if($nocomments=~m/[^a-zA-Z0-9_]+CONFIG_([a-zA-Z0-9_]+)[^a-zA-Z0-9_]/){printfSTDERR"$filename:$lineno: leaks CONFIG_$1 to userspace where it is not valid\n";}}
I think this actually isn't a good idea, but I can't find anything
outside of the kernel that's using this so I'm going to hide it.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
arch/x86/include/uapi/asm/auxvec.h | 2 ++
1 file changed, 2 insertions(+)
I don't think this was ever meant to be exposed to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/raw.h | 2 ++
1 file changed, 2 insertions(+)
I don't think this was ever intended to be exposed to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/pktcdvd.h | 2 ++
1 file changed, 2 insertions(+)
I'm actually not sure what to do here: if this enum is meant to be
used by userspace, then it has to be the same regardless of kernel
configuration. One option would be to have the kernel expose all the
values to userspace and then map them internally if
CONFIG_HAVE_MIXED_BREAKPOINT_REGS isn't set, but that feels like it'd
be more invasive.
Here I took the simple and fail-fast route to hide all the
definitions.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
I'm not sure what this is, but it doesn't feel like something that
should be exposed to userspace here. I'm assuming this file was
exposed for the structure in it, which doesn't depend on
MAX_SHARED_LIBS.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/flat.h | 2 ++
1 file changed, 2 insertions(+)
This doesn't make any sense to expose to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/eventpoll.h | 3 +++
1 file changed, 3 insertions(+)
This one scares me: while I can't find any system calls that directly
take this as an argument, a comment in <linux/ptrace.h>
"
Generic ptrace interface that exports the architecture specific
regsets using the corresponding NT_* types (which are also used in
the core dump). Please note that the NT_PRSTATUS note type in a
core dump contains a full 'struct elf_prstatus'. But the
user_regset for NT_PRSTATUS contains just the elf_gregset_t that is
the pr_reg field of 'struct elf_prstatus'. For all the other
user_regset flavors, the user_regset layout and the ELF core dump
note payload are exactly the same layout.
"
seems to indicate that it's possible to see this sometimes. Since
this would only be visible to userspace in a somewhat convoluted
manner, I'm going to try and keep it as it was.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/elfcore.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -60,7 +60,7 @@ struct elf_prstatuslongpr_instr;/* Current instruction */#endifelf_gregset_tpr_reg;/* GP registers */-#ifdef CONFIG_BINFMT_ELF_FDPIC+#if defined(__KERNEL__) && defined(CONFIG_BINFMT_ELF_FDPIC)/* When using FDPIC, the loadmap addresses need to be communicated*toGDBinorderforGDBtodothenecessaryrelocations.The*fields(below)usedtocommunicatethisinformationareplaced
This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so
userspace wouldn't actually ever see it. While I could have kept
hiding it, the man pages seem to indicate that MAP_UNINITIALIZED
should be visible:
mmap(2)
MAP_UNINITIALIZED (since Linux 2.6.33)
Don't clear anonymous pages. This flag is intended to improve
performance on embedded devices. This flag is honored only if the
kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED
option. Because of the security implications, that option is
normally enabled only on embedded devices (i.e., devices where one
has complete control of the contents of user memory).
and since the only time it shows up in my /usr/include is in this
header I believe this should have been visible to userspace (as
non-zero, which wouldn't do anything when or'd into the flags) all
along.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/mman-common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -19,7 +19,7 @@#define MAP_TYPE 0x0f /* Mask for type of mapping */#define MAP_FIXED 0x10 /* Interpret addr exactly */#define MAP_ANONYMOUS 0x20 /* don't use a file */-#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED+#if !defined(__KERNEL__) || defined(CONFIG_MMAP_ALLOW_UNINITIALIZED)# define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be uninitialized */#else# define MAP_UNINITIALIZED 0x0 /* Don't support this flag */
This used to just be behind an #ifdef COMPAT_COMPAT, so most of
userspace wouldn't have seen the definition before. This change just
makes the __KERNEL__ part explicit to quiet the header checker.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/atmdev.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -100,7 +100,7 @@ struct atm_dev_stats {/* use backend to make new if */#define ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)/* add party to p2mp call */-#ifdef CONFIG_COMPAT+#if defined(__KERNEL__) && defined(CONFIG_COMPAT)/* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */#define COMPAT_ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)#endif
I think this change actually doesn't do anything: __NR_fork was still
being defined either way, and on my machine fork() in <unistd.h> comes
from libc.
That said, I don't think there's any way to determine this
automatically, so this at least quiets the checker.
Signed-off-by: Palmer Dabbelt <redacted>
Reviewed-by: Andrew Waterman <waterman-aFE07iDfcCIb0cFwG/AQJIdd74u8MsAO@public.gmane.org>
Reviewed-by: Albert Ou <aou-aFE07iDfcCIb0cFwG/AQJIdd74u8MsAO@public.gmane.org>
---
include/uapi/asm-generic/unistd.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: David Howells <dhowells@redhat.com> Date: 2015-09-10 11:15:39
Rather than iterating through all the rest of your patches and saying the same
thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
to exclude it from userspace's use, then it should be transferred to the
non-UAPI variant of that header (which should #include the UAPI variant).
David
From: David Howells <dhowells@redhat.com> Date: 2015-09-10 11:18:16
David Howells [off-list ref] wrote:
Rather than iterating through all the rest of your patches and saying the same
thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
to exclude it from userspace's use, then it should be transferred to the
non-UAPI variant of that header (which should #include the UAPI variant).
I should mention that there is the odd case where this is difficult to
achieve. See include/uapi/linux/acct.h for an example...
David
On Thu, 10 Sep 2015 04:18:05 PDT (-0700), dhowells@redhat.com wrote:
David Howells [off-list ref] wrote:
quoted
Rather than iterating through all the rest of your patches and saying the same
thing, if there's something in a UAPI header that needs wrapping in __KERNEL__
to exclude it from userspace's use, then it should be transferred to the
non-UAPI variant of that header (which should #include the UAPI variant).
I should mention that there is the odd case where this is difficult to
achieve. See include/uapi/linux/acct.h for an example...
OK, sorry about that. I'm submitting a v3 that should fix these
problems.
This patch set used to be called "[PATCH] Remove #ifdef CONFIG_64BIT
from all asm-generic/fcntl.h".
This is version 3 of the patch set. I think things are kind of
cleaning up, but there's still some questions I have:
* #4 might be dangerous, but I think this is the best way to do it
because otherwise arch maintainers will have to #define this to
something. Since Xtensa already had exactly the same bug as the
asm-generic header did, I think it'd be best to avoid those if
possible.
I'm OK changing this to define the value to 0 by default (though
thinking now, I think it'd be better to not define it by default),
since most kernels aren't going to have MAP_UNINITIALIZED do
anything anyway.
* #5 is big and I don't really understand what's going on, so it's
probably broken somewhere.
Also, there's some checkpatch warnings that I purposely haven't fixed:
* In #5, there's a checkpatch.pl complaint about same-line struct
braces. I've left that in to match the rest of the file, but I can
also fix all of them if that would be better.
* In #10, there's a checkpatch.pl complaint about a split printk
format string, but that was there before and I don't see a way to
make it a lot cleaner, so I'm leaving it alone.
* In #13, there's two checkpatch.pl complains. One is about block
comments that is bogus, and one is about long lines but there's
more of those so I think it's OK.
Thanks to everyone who looked at v2!
Changes since v2 ([off-list ref])
* Patch set renamed.
* #2 is rewritten to use sys_ni.c instead of an #ifdef
* #3, #6, #8, #9, #10, and #11 no longer use "#ifdef __KERNEL__" but
have instead moved the offending lines to the correct, kernel-only
files.
* #4 has been rewritten to always define MAP_UNINITIALIZED to
non-zero, rather than defining it to zero when in userspace.
* #5 got a whole lot longer -- rather than just always hiding these
fields from userspace, there is now a second "struct
elf_fdpic_prstatus" structure. This should allow userspace to
parse core dumps correctly.
* Rebased onto 9c488de24f7264f08d341024bffdd637b4d04c96.
Changes since v1 ([off-list ref])
* All patches but #1 were added.
When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops. Since there's not compat
layer for these, this causes fcntl to bail out.
It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/fcntl.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops. Since there's not compat
layer for these, this causes fcntl to bail out.
It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/fcntl.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
I think this change actually doesn't do anything: __NR_fork was still
being defined either way, and on my machine fork() in <unistd.h> comes
from libc.
This just moves to the standard mechanism for defining syscalls that
aren't implemented instead, which has the side-effect of no longer
having an #ifdef CONFIG_* in a user-visible header.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/unistd.h | 4 ----
kernel/sys_ni.c | 1 +
2 files changed, 1 insertion(+), 4 deletions(-)
I think this change actually doesn't do anything: __NR_fork was still
being defined either way, and on my machine fork() in <unistd.h> comes
from libc.
This just moves to the standard mechanism for defining syscalls that
aren't implemented instead, which has the side-effect of no longer
having an #ifdef CONFIG_* in a user-visible header.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/unistd.h | 4 ----
kernel/sys_ni.c | 1 +
2 files changed, 1 insertion(+), 4 deletions(-)
This used to be behind an #ifdef COMPAT_COMPAT, so most of userspace
wouldn't have seen the definition before. Unfortunately this header
file became visible to userspace, so the definition has instead been
moved to net/atm/svc.c (the only user).
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/atmdev.h | 4 ----
net/atm/svc.c | 5 +++++
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -100,10 +100,6 @@ struct atm_dev_stats {/* use backend to make new if */#define ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)/* add party to p2mp call */-#ifdef CONFIG_COMPAT-/* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */-#define COMPAT_ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)-#endif#define ATM_DROPPARTY _IOW('a', ATMIOC_SPECIAL+5,int)/* drop party from p2mp call */
This used to be behind an #ifdef COMPAT_COMPAT, so most of userspace
wouldn't have seen the definition before. Unfortunately this header
file became visible to userspace, so the definition has instead been
moved to net/atm/svc.c (the only user).
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/atmdev.h | 4 ----
net/atm/svc.c | 5 +++++
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -100,10 +100,6 @@ struct atm_dev_stats {/* use backend to make new if */#define ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)/* add party to p2mp call */-#ifdef CONFIG_COMPAT-/* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */-#define COMPAT_ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)-#endif#define ATM_DROPPARTY _IOW('a', ATMIOC_SPECIAL+5,int)/* drop party from p2mp call */
This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so
userspace wouldn't actually ever see it be non-zero. While I could
have kept hiding it, the man pages seem to indicate that
MAP_UNINITIALIZED should be visible:
mmap(2)
MAP_UNINITIALIZED (since Linux 2.6.33)
Don't clear anonymous pages. This flag is intended to improve
performance on embedded devices. This flag is honored only if the
kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED
option. Because of the security implications, that option is
normally enabled only on embedded devices (i.e., devices where one
has complete control of the contents of user memory).
and since the only time it shows up in my /usr/include is in this
header I believe this should have been visible to userspace (as
non-zero, which wouldn't do anything when or'd into the flags) all
along.
This change also applies to the xtensa version of this definition,
whic is the same as the generic one.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
arch/xtensa/include/uapi/asm/mman.h | 4 +---
include/uapi/asm-generic/mman-common.h | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
@@ -55,11 +55,9 @@#define MAP_NONBLOCK 0x20000 /* do not block on IO */#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */#define MAP_HUGETLB 0x80000 /* create a huge page mapping */-#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED+#ifndef MAP_UNINITIALIZED# define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be*uninitialized*/-#else-# define MAP_UNINITIALIZED 0x0 /* Don't support this flag */#endif/*
This used to be hidden behind CONFIG_MMAP_ALLOW_UNINITIALIZED, so
userspace wouldn't actually ever see it be non-zero. While I could
have kept hiding it, the man pages seem to indicate that
MAP_UNINITIALIZED should be visible:
mmap(2)
MAP_UNINITIALIZED (since Linux 2.6.33)
Don't clear anonymous pages. This flag is intended to improve
performance on embedded devices. This flag is honored only if the
kernel was configured with the CONFIG_MMAP_ALLOW_UNINITIALIZED
option. Because of the security implications, that option is
normally enabled only on embedded devices (i.e., devices where one
has complete control of the contents of user memory).
and since the only time it shows up in my /usr/include is in this
header I believe this should have been visible to userspace (as
non-zero, which wouldn't do anything when or'd into the flags) all
along.
This change also applies to the xtensa version of this definition,
whic is the same as the generic one.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
arch/xtensa/include/uapi/asm/mman.h | 4 +---
include/uapi/asm-generic/mman-common.h | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
@@ -55,11 +55,9 @@#define MAP_NONBLOCK 0x20000 /* do not block on IO */#define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */#define MAP_HUGETLB 0x80000 /* create a huge page mapping */-#ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED+#ifndef MAP_UNINITIALIZED# define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be*uninitialized*/-#else-# define MAP_UNINITIALIZED 0x0 /* Don't support this flag */#endif/*
This is a pretty big change that I haven't tested any, which worries
me a lot. The general idea here is that "struct elf_prstatus" can be
visible to userspace (from <linux/ptrace.h>)
"
Generic ptrace interface that exports the architecture specific
regsets using the corresponding NT_* types (which are also used in
the core dump). Please note that the NT_PRSTATUS note type in a
core dump contains a full 'struct elf_prstatus'. But the
user_regset for NT_PRSTATUS contains just the elf_gregset_t that is
the pr_reg field of 'struct elf_prstatus'. For all the other
user_regset flavors, the user_regset layout and the ELF core dump
note payload are exactly the same layout.
"
so it shouldn't have an "#ifdef CONFIG_" in there.
This splits the structure into two different structures, one still
named "struct elf_prstatus", and one for ELF FDPIC that is named
"struct elf_fdpic_prstatus" -- the idea here is that most users are
standard ELF, so that's the name that didn't change.
I tried to fix all the users of "struct elf_prstatus" that should now
be using "struct elf_fdpic_prstatus". The only testing I did here was
to build a Blackfin defconfig with "struct elf_prstatus" not defined,
and to build an x86 defconfig with "struct elf_fdpic_prstatus" not
defined.
Note that this fails checkpatch.pl with a complaint about wanting open
braces on the same line as struct definitions. The existing struct
definitions in this file have the brace a line afterwards, so I'm
going to leave this alone.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/binfmt_elf_fdpic.c | 6 +++---
fs/proc/kcore.c | 16 ++++++++++++++++
include/linux/kexec.h | 4 ++++
include/uapi/linux/elfcore.h | 37 +++++++++++++++++++++++++++++++++++--
kernel/kexec_core.c | 4 ++++
5 files changed, 62 insertions(+), 5 deletions(-)
@@ -387,10 +395,18 @@ static void elf_kcore_store_hdr(char *bufp, int nphdr, int dataoff)/* set up the process status */notes[0].name=CORE_STR;notes[0].type=NT_PRSTATUS;+#ifdef CONFIG_BINFMT_ELF_FDPIC+notes[0].datasz=sizeof(structelf_fdpic_prstatus);+#elsenotes[0].datasz=sizeof(structelf_prstatus);+#endifnotes[0].data=&prstatus;+#ifdef CONFIG_BINFMT_ELF_FDPIC+memset(&prstatus,0,sizeof(structelf_fdpic_prstatus));+#elsememset(&prstatus,0,sizeof(structelf_prstatus));+#endifnhdr->p_filesz=notesize(¬es[0]);bufp=storenote(¬es[0],bufp);
@@ -60,7 +60,40 @@ struct elf_prstatuslongpr_instr;/* Current instruction */#endifelf_gregset_tpr_reg;/* GP registers */-#ifdef CONFIG_BINFMT_ELF_FDPIC+intpr_fpvalid;/* True if math co-processor being used. */+};++/* Architectures that set CONFIG_BINFMT_ELF_FDPIC use this instead of+*"struct elf_prstatus".+*/+structelf_fdpic_prstatus+{+#if 0+longpr_flags;/* XXX Process flags */+shortpr_why;/* XXX Reason for process halt */+shortpr_what;/* XXX More detailed reason */+#endif+structelf_siginfopr_info;/* Info associated with signal */+shortpr_cursig;/* Current signal */+unsignedlongpr_sigpend;/* Set of pending signals */+unsignedlongpr_sighold;/* Set of held signals */+#if 0+structsigaltstackpr_altstack;/* Alternate stack info */+structsigactionpr_action;/* Signal action for current sig */+#endif+pid_tpr_pid;+pid_tpr_ppid;+pid_tpr_pgrp;+pid_tpr_sid;+structtimevalpr_utime;/* User time */+structtimevalpr_stime;/* System time */+structtimevalpr_cutime;/* Cumulative user time */+structtimevalpr_cstime;/* Cumulative system time */+#if 0+longpr_instr;/* Current instruction */+#endif+elf_gregset_tpr_reg;/* GP registers */+/* When using FDPIC, the loadmap addresses need to be communicated*toGDBinorderforGDBtodothenecessaryrelocations.The*fields(below)usedtocommunicatethisinformationareplaced
@@ -69,7 +102,7 @@ struct elf_prstatus*/unsignedlongpr_exec_fdpic_loadmap;unsignedlongpr_interp_fdpic_loadmap;-#endif+intpr_fpvalid;/* True if math co-processor being used. */};
This is a pretty big change that I haven't tested any, which worries
me a lot. The general idea here is that "struct elf_prstatus" can be
visible to userspace (from <linux/ptrace.h>)
"
Generic ptrace interface that exports the architecture specific
regsets using the corresponding NT_* types (which are also used in
the core dump). Please note that the NT_PRSTATUS note type in a
core dump contains a full 'struct elf_prstatus'. But the
user_regset for NT_PRSTATUS contains just the elf_gregset_t that is
the pr_reg field of 'struct elf_prstatus'. For all the other
user_regset flavors, the user_regset layout and the ELF core dump
note payload are exactly the same layout.
"
so it shouldn't have an "#ifdef CONFIG_" in there.
This splits the structure into two different structures, one still
named "struct elf_prstatus", and one for ELF FDPIC that is named
"struct elf_fdpic_prstatus" -- the idea here is that most users are
standard ELF, so that's the name that didn't change.
I tried to fix all the users of "struct elf_prstatus" that should now
be using "struct elf_fdpic_prstatus". The only testing I did here was
to build a Blackfin defconfig with "struct elf_prstatus" not defined,
and to build an x86 defconfig with "struct elf_fdpic_prstatus" not
defined.
Note that this fails checkpatch.pl with a complaint about wanting open
braces on the same line as struct definitions. The existing struct
definitions in this file have the brace a line afterwards, so I'm
going to leave this alone.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/binfmt_elf_fdpic.c | 6 +++---
fs/proc/kcore.c | 16 ++++++++++++++++
include/linux/kexec.h | 4 ++++
include/uapi/linux/elfcore.h | 37 +++++++++++++++++++++++++++++++++++--
kernel/kexec_core.c | 4 ++++
5 files changed, 62 insertions(+), 5 deletions(-)
@@ -387,10 +395,18 @@ static void elf_kcore_store_hdr(char *bufp, int nphdr, int dataoff)/* set up the process status */notes[0].name=CORE_STR;notes[0].type=NT_PRSTATUS;+#ifdef CONFIG_BINFMT_ELF_FDPIC+notes[0].datasz=sizeof(structelf_fdpic_prstatus);+#elsenotes[0].datasz=sizeof(structelf_prstatus);+#endifnotes[0].data=&prstatus;+#ifdef CONFIG_BINFMT_ELF_FDPIC+memset(&prstatus,0,sizeof(structelf_fdpic_prstatus));+#elsememset(&prstatus,0,sizeof(structelf_prstatus));+#endifnhdr->p_filesz=notesize(¬es[0]);bufp=storenote(¬es[0],bufp);
@@ -60,7 +60,40 @@ struct elf_prstatuslongpr_instr;/* Current instruction */#endifelf_gregset_tpr_reg;/* GP registers */-#ifdef CONFIG_BINFMT_ELF_FDPIC+intpr_fpvalid;/* True if math co-processor being used. */+};++/* Architectures that set CONFIG_BINFMT_ELF_FDPIC use this instead of+*"struct elf_prstatus".+*/+structelf_fdpic_prstatus+{+#if 0+longpr_flags;/* XXX Process flags */+shortpr_why;/* XXX Reason for process halt */+shortpr_what;/* XXX More detailed reason */+#endif+structelf_siginfopr_info;/* Info associated with signal */+shortpr_cursig;/* Current signal */+unsignedlongpr_sigpend;/* Set of pending signals */+unsignedlongpr_sighold;/* Set of held signals */+#if 0+structsigaltstackpr_altstack;/* Alternate stack info */+structsigactionpr_action;/* Signal action for current sig */+#endif+pid_tpr_pid;+pid_tpr_ppid;+pid_tpr_pgrp;+pid_tpr_sid;+structtimevalpr_utime;/* User time */+structtimevalpr_stime;/* System time */+structtimevalpr_cutime;/* Cumulative user time */+structtimevalpr_cstime;/* Cumulative system time */+#if 0+longpr_instr;/* Current instruction */+#endif+elf_gregset_tpr_reg;/* GP registers */+/* When using FDPIC, the loadmap addresses need to be communicated*toGDBinorderforGDBtodothenecessaryrelocations.The*fields(below)usedtocommunicatethisinformationareplaced
@@ -69,7 +102,7 @@ struct elf_prstatus*/unsignedlongpr_exec_fdpic_loadmap;unsignedlongpr_interp_fdpic_loadmap;-#endif+intpr_fpvalid;/* True if math co-processor being used. */};
This doesn't make any sense to expose to userspace, so it's been moved
to the one user. This was introduced by commit 95f19f658ce1 ("epoll:
drop EPOLLWAKEUP if PM_SLEEP is disabled").
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/eventpoll.c | 13 +++++++++++++
include/uapi/linux/eventpoll.h | 12 ------------
2 files changed, 13 insertions(+), 12 deletions(-)
This doesn't make any sense to expose to userspace, so it's been moved
to the one user. This was introduced by commit 95f19f658ce1 ("epoll:
drop EPOLLWAKEUP if PM_SLEEP is disabled").
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/eventpoll.c | 13 +++++++++++++
include/uapi/linux/eventpoll.h | 12 ------------
2 files changed, 13 insertions(+), 12 deletions(-)
Nothing else in the kernel defines this, and this header is visible to
userspace. Rather than hiding it in an #ifdef, I think it's sane to
just make this visible to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/fb.h | 3 ---
1 file changed, 3 deletions(-)
Nothing else in the kernel defines this, and this header is visible to
userspace. Rather than hiding it in an #ifdef, I think it's sane to
just make this visible to userspace.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/fb.h | 3 ---
1 file changed, 3 deletions(-)
I'm not sure what this is, but it doesn't feel like something that
should be exposed to userspace here. I'm assuming this file was
exposed for the structure in it, which doesn't depend on
MAX_SHARED_LIBS.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/binfmt_flat.c | 6 ++++++
include/uapi/linux/flat.h | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
I'm not sure what this is, but it doesn't feel like something that
should be exposed to userspace here. I'm assuming this file was
exposed for the structure in it, which doesn't depend on
MAX_SHARED_LIBS.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
fs/binfmt_flat.c | 6 ++++++
include/uapi/linux/flat.h | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). Since
there's only one kernel user, it's been moved to that file.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 10 ----------
kernel/events/hw_breakpoint.c | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-09-15 08:06:07
On Mon, Sep 14, 2015 at 03:50:43PM -0700, Palmer Dabbelt wrote:
quoted hunk
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). Since
there's only one kernel user, it's been moved to that file.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 10 ----------
kernel/events/hw_breakpoint.c | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:
perf_event_attr::bp_type
So removing them will break things.
Frederic?
On Tue, 15 Sep 2015 01:06:07 PDT (-0700), peterz@infradead.org wrote:
On Mon, Sep 14, 2015 at 03:50:43PM -0700, Palmer Dabbelt wrote:
quoted
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). Since
there's only one kernel user, it's been moved to that file.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 10 ----------
kernel/events/hw_breakpoint.c | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:
perf_event_attr::bp_type
So removing them will break things.
Frederic?
perf_event_open(2) says
bp_type (since Linux 2.6.33)
This chooses the breakpoint type. It is one of:
HW_BREAKPOINT_EMPTY
No breakpoint.
HW_BREAKPOINT_R
Count when we read the memory location.
HW_BREAKPOINT_W
Count when we write the memory location.
HW_BREAKPOINT_RW
Count when we read or write the memory location.
HW_BREAKPOINT_X
Count when we execute code at the memory location.
The values can be combined via a bitwise or, but the combination
of HW_BREAKPOINT_R or HW_BREAKPOINT_W with HW_BREAKPOINT_X is
not allowed.
so I think removing this enum from userspace is OK. Did I miss
something?
From: Peter Zijlstra <peterz@infradead.org> Date: 2015-09-15 19:39:10
On Tue, Sep 15, 2015 at 11:40:11AM -0700, Palmer Dabbelt wrote:
On Tue, 15 Sep 2015 01:06:07 PDT (-0700), peterz@infradead.org wrote:
quoted
On Mon, Sep 14, 2015 at 03:50:43PM -0700, Palmer Dabbelt wrote:
quoted
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). Since
there's only one kernel user, it's been moved to that file.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 10 ----------
kernel/events/hw_breakpoint.c | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:
perf_event_attr::bp_type
So removing them will break things.
Frederic?
perf_event_open(2) says
bp_type (since Linux 2.6.33)
This chooses the breakpoint type. It is one of:
HW_BREAKPOINT_EMPTY
No breakpoint.
HW_BREAKPOINT_R
Count when we read the memory location.
HW_BREAKPOINT_W
Count when we write the memory location.
HW_BREAKPOINT_RW
Count when we read or write the memory location.
HW_BREAKPOINT_X
Count when we execute code at the memory location.
The values can be combined via a bitwise or, but the combination
of HW_BREAKPOINT_R or HW_BREAKPOINT_W with HW_BREAKPOINT_X is
not allowed.
so I think removing this enum from userspace is OK. Did I miss
something?
Nah, could've just been me not being awake. Unless Frederic says
otherwise I'll chalk it up to not having drank enough morning juice.
This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:
perf_event_attr::bp_type
So removing them will break things.
Frederic?
If user space actually relies on the definition from this header file,
then it will use the wrong one on x86 and get 'TYPE_DATA = 1', while the
kernel uses 'TYPE_DATA = 0'.
That seems unlikely to work, so I suspect it gets a different definition.
If it uses this definition and it does work, we can probably use
#if defined(__KERNEL__) && defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
but that requires a comment explaining exactly why that works.
Arnd
From: David Howells <dhowells@redhat.com> Date: 2015-09-17 10:28:20
Arnd Bergmann [off-list ref] wrote:
That seems unlikely to work, so I suspect it gets a different definition.
If it uses this definition and it does work, we can probably use
#if defined(__KERNEL__) && defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
but that requires a comment explaining exactly why that works.
Are you suggesting wrapping the entire construct in this within the UAPI
header?
David
This is rather unfortunate; you are correct that the naming is too
generic (and I tend to agree), but I think these values are required by
userspace to fill out:
perf_event_attr::bp_type
So removing them will break things.
Frederic?
If user space actually relies on the definition from this header file,
then it will use the wrong one on x86 and get 'TYPE_DATA = 1', while the
kernel uses 'TYPE_DATA = 0'.
That seems unlikely to work, so I suspect it gets a different definition.
If it uses this definition and it does work, we can probably use
#if defined(__KERNEL__) && defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
but that requires a comment explaining exactly why that works.
I think this TYPE_DATA/TYPE_INST can be safely removed from uapi. This is only
about internal kernel code. Userspace only relies on HW_BREAKPOINT_[R/W/X]
to tell about the nature of the breakpoint.
If userspace ever relies on it, which I have no idea why, it even needs to define
CONFIG_HAVE_MIXED_BREAKPOINTS_REGS. No really there shouldn't be any user of that outside
the kernel.
Thanks.
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). Since
there's only one kernel user, it's been moved to that file.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/linux/hw_breakpoint.h | 10 ----------
kernel/events/hw_breakpoint.c | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
I don't think this was ever intended to be exposed to userspace, but
it did require an "#ifdef CONFIG_*". Since the name is kind of
generic and was only used in one place, I've moved the definition to
the one user.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
drivers/block/pktcdvd.c | 11 +++++++++++
include/uapi/linux/pktcdvd.h | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)
I don't think this was ever intended to be exposed to userspace, but
it did require an "#ifdef CONFIG_*". Since the name is kind of
generic and was only used in one place, I've moved the definition to
the one user.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
drivers/block/pktcdvd.c | 11 +++++++++++
include/uapi/linux/pktcdvd.h | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)
While I don't think this was ever meant to be exposed to userspace, if
anyone is using it then this will at least provide a correct (if
unlikely) definition.
MAX_RAW_MINORS used to be used in the kernel, where it's been replaced
with CONFIG_MAX_RAW_MINORS.
Note that there's a checkpatch.pl warning about a split config string
here, but I've left that alone.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
drivers/char/raw.c | 7 ++++---
include/uapi/linux/raw.h | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -36,7 +36,7 @@ static struct raw_device_data *raw_devices;staticDEFINE_MUTEX(raw_mutex);staticconststructfile_operationsraw_ctl_fops;/* forward declaration */-staticintmax_raw_minors=MAX_RAW_MINORS;+staticintmax_raw_minors=CONFIG_MAX_RAW_MINORS;module_param(max_raw_minors,int,0);MODULE_PARM_DESC(max_raw_minors,"Maximum number of raw devices (1-65536)");
@@ -317,8 +317,9 @@ static int __init raw_init(void)if(max_raw_minors<1||max_raw_minors>65536){printk(KERN_WARNING"raw: invalid max_raw_minors (must be"-" between 1 and 65536), using %d\n",MAX_RAW_MINORS);-max_raw_minors=MAX_RAW_MINORS;+" between 1 and 65536), using %d\n",+CONFIG_MAX_RAW_MINORS);+max_raw_minors=CONFIG_MAX_RAW_MINORS;}raw_devices=vzalloc(sizeof(structraw_device_data)*max_raw_minors);
While I don't think this was ever meant to be exposed to userspace, if
anyone is using it then this will at least provide a correct (if
unlikely) definition.
MAX_RAW_MINORS used to be used in the kernel, where it's been replaced
with CONFIG_MAX_RAW_MINORS.
Note that there's a checkpatch.pl warning about a split config string
here, but I've left that alone.
Signed-off-by: Palmer Dabbelt <palmer@dabbelt.com>
Reviewed-by: Andrew Waterman <redacted>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
drivers/char/raw.c | 7 ++++---
include/uapi/linux/raw.h | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -36,7 +36,7 @@ static struct raw_device_data *raw_devices;staticDEFINE_MUTEX(raw_mutex);staticconststructfile_operationsraw_ctl_fops;/* forward declaration */-staticintmax_raw_minors=MAX_RAW_MINORS;+staticintmax_raw_minors=CONFIG_MAX_RAW_MINORS;module_param(max_raw_minors,int,0);MODULE_PARM_DESC(max_raw_minors,"Maximum number of raw devices (1-65536)");
@@ -317,8 +317,9 @@ static int __init raw_init(void)if(max_raw_minors<1||max_raw_minors>65536){printk(KERN_WARNING"raw: invalid max_raw_minors (must be"-" between 1 and 65536), using %d\n",MAX_RAW_MINORS);-max_raw_minors=MAX_RAW_MINORS;+" between 1 and 65536), using %d\n",+CONFIG_MAX_RAW_MINORS);+max_raw_minors=CONFIG_MAX_RAW_MINORS;}raw_devices=vzalloc(sizeof(structraw_device_data)*max_raw_minors);