diff mbox

Fix sys/procfs.h pr_uid, pr_gid type (bug 23649)

Message ID alpine.DEB.2.21.1809132048480.10670@digraph.polyomino.org.uk
State New, archived
Headers

Commit Message

Joseph Myers Sept. 13, 2018, 8:49 p.m. UTC 
  As noted in
<https://sourceware.org/ml/libc-alpha/2018-09/msg00178.html>, glibc's
sys/procfs.h headers for microblaze, mips (n64), nios2 and riscv have
incorrect types for the pr_uid and pr_gid members of struct
elf_prpsinfo (as does the generic Linux version, but nothing uses
that).

This patch fixes those headers to use unsigned int.  The generic Linux
version is also fixed, but I do *not* recommend making new
architectures use it yet.  Rather, I think it should be reworked to
look more like a copy of the AArch64 version, but with a new
<bits/procfs.h> header included to provide register set definitions;
<bits/procfs.h> would then be architecture-specific while many
architectures could use the generic <sys/procfs.h>.  This fix is
deliberately separate from any reworking to use a generic header more,
since it's possible there could be uses for backporting this fix but
not for backporting a subsequent cleanup.

Tested with build-many-glibcs.py.  This of course doesn't provide much
validation of the structure layout; if the Linux kernel is fixed so
that "#include <linux/elfcore.h>" actually compiles with the headers
from "make headers_install" (and if the layout in both headers is
meant to be the same, whatever ABI we are building for), I have a test
that can be added to glibc to check the layout against that from the
Linux kernel.

2018-09-13  Joseph Myers  <joseph@codesourcery.com>

	[BZ #23649]
	* sysdeps/unix/sysv/linux/microblaze/sys/procfs.h (struct
	elf_prpsinfo): Use unsigned int for pr_uid and pr_gid.
	* sysdeps/unix/sysv/linux/mips/sys/procfs.h (struct elf_prpsinfo):
	Likewise.
	* sysdeps/unix/sysv/linux/nios2/sys/procfs.h (struct
	elf_prpsinfo): Likewise.
	* sysdeps/unix/sysv/linux/riscv/sys/procfs.h (struct
	elf_prpsinfo): Likewise.
	* sysdeps/unix/sysv/linux/sys/procfs.h (struct elf_prpsinfo):
	Likewise.

Comments

Florian Weimer Sept. 14, 2018, 8:01 a.m. UTC | #1 
On 09/13/2018 10:49 PM, Joseph Myers wrote:
> 2018-09-13  Joseph Myers<joseph@codesourcery.com>
> 
> 	[BZ #23649]
> 	* sysdeps/unix/sysv/linux/microblaze/sys/procfs.h (struct
> 	elf_prpsinfo): Use unsigned int for pr_uid and pr_gid.
> 	* sysdeps/unix/sysv/linux/mips/sys/procfs.h (struct elf_prpsinfo):
> 	Likewise.
> 	* sysdeps/unix/sysv/linux/nios2/sys/procfs.h (struct
> 	elf_prpsinfo): Likewise.
> 	* sysdeps/unix/sysv/linux/riscv/sys/procfs.h (struct
> 	elf_prpsinfo): Likewise.
> 	* sysdeps/unix/sysv/linux/sys/procfs.h (struct elf_prpsinfo):
> 	Likewise.

The changes look okay to me.  Does this need a NEWS entry?  After all, 
the struct size changes with this patch, as far as I can tell.

Thanks,
Florian
Arnd Bergmann Sept. 14, 2018, 11:30 a.m. UTC | #2 
On Thu, Sep 13, 2018 at 10:49 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> As noted in
> <https://sourceware.org/ml/libc-alpha/2018-09/msg00178.html>, glibc's
> sys/procfs.h headers for microblaze, mips (n64), nios2 and riscv have
> incorrect types for the pr_uid and pr_gid members of struct
> elf_prpsinfo (as does the generic Linux version, but nothing uses
> that).
>
> This patch fixes those headers to use unsigned int.  The generic Linux
> version is also fixed, but I do *not* recommend making new
> architectures use it yet.  Rather, I think it should be reworked to
> look more like a copy of the AArch64 version, but with a new
> <bits/procfs.h> header included to provide register set definitions;
> <bits/procfs.h> would then be architecture-specific while many
> architectures could use the generic <sys/procfs.h>.  This fix is
> deliberately separate from any reworking to use a generic header more,
> since it's possible there could be uses for backporting this fix but
> not for backporting a subsequent cleanup.
>
> Tested with build-many-glibcs.py.  This of course doesn't provide much
> validation of the structure layout; if the Linux kernel is fixed so
> that "#include <linux/elfcore.h>" actually compiles with the headers
> from "make headers_install" (and if the layout in both headers is
> meant to be the same, whatever ABI we are building for), I have a test
> that can be added to glibc to check the layout against that from the
> Linux kernel.
>
> 2018-09-13  Joseph Myers  <joseph@codesourcery.com>

Looks all good to me, thanks for addressing this!

I've had a look at including linux/elfcore.h from user space,
and what it would take to get it working again. I've got a
prototype patch now (x86 only) that I'll post for futher discussion.

       Arnd
Joseph Myers Sept. 14, 2018, 1:22 p.m. UTC | #3 
On Fri, 14 Sep 2018, Florian Weimer wrote:

> On 09/13/2018 10:49 PM, Joseph Myers wrote:
> > 2018-09-13  Joseph Myers<joseph@codesourcery.com>
> > 
> > 	[BZ #23649]
> > 	* sysdeps/unix/sysv/linux/microblaze/sys/procfs.h (struct
> > 	elf_prpsinfo): Use unsigned int for pr_uid and pr_gid.
> > 	* sysdeps/unix/sysv/linux/mips/sys/procfs.h (struct elf_prpsinfo):
> > 	Likewise.
> > 	* sysdeps/unix/sysv/linux/nios2/sys/procfs.h (struct
> > 	elf_prpsinfo): Likewise.
> > 	* sysdeps/unix/sysv/linux/riscv/sys/procfs.h (struct
> > 	elf_prpsinfo): Likewise.
> > 	* sysdeps/unix/sysv/linux/sys/procfs.h (struct elf_prpsinfo):
> > 	Likewise.
> 
> The changes look okay to me.  Does this need a NEWS entry?  After all, the
> struct size changes with this patch, as far as I can tell.

I've committed it with the following NEWS entry.

* The type of the pr_uid and pr_gid members of struct elf_prpsinfo, defined
  in <sys/procfs.h>, has been corrected to match the type actually used by
  the Linux kernel.  This affects the size and layout of that structure on
  MicroBlaze, MIPS (n64 ABI only), Nios II and RISC-V.
Florian Weimer Sept. 14, 2018, 1:23 p.m. UTC | #4 
On 09/14/2018 03:22 PM, Joseph Myers wrote:
> On Fri, 14 Sep 2018, Florian Weimer wrote:
> 
>> On 09/13/2018 10:49 PM, Joseph Myers wrote:
>>> 2018-09-13  Joseph Myers<joseph@codesourcery.com>
>>>
>>> 	[BZ #23649]
>>> 	* sysdeps/unix/sysv/linux/microblaze/sys/procfs.h (struct
>>> 	elf_prpsinfo): Use unsigned int for pr_uid and pr_gid.
>>> 	* sysdeps/unix/sysv/linux/mips/sys/procfs.h (struct elf_prpsinfo):
>>> 	Likewise.
>>> 	* sysdeps/unix/sysv/linux/nios2/sys/procfs.h (struct
>>> 	elf_prpsinfo): Likewise.
>>> 	* sysdeps/unix/sysv/linux/riscv/sys/procfs.h (struct
>>> 	elf_prpsinfo): Likewise.
>>> 	* sysdeps/unix/sysv/linux/sys/procfs.h (struct elf_prpsinfo):
>>> 	Likewise.
>>
>> The changes look okay to me.  Does this need a NEWS entry?  After all, the
>> struct size changes with this patch, as far as I can tell.
> 
> I've committed it with the following NEWS entry.
> 
> * The type of the pr_uid and pr_gid members of struct elf_prpsinfo, defined
>    in <sys/procfs.h>, has been corrected to match the type actually used by
>    the Linux kernel.  This affects the size and layout of that structure on
>    MicroBlaze, MIPS (n64 ABI only), Nios II and RISC-V.

Thanks, looks good.

Florian
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/microblaze/sys/procfs.h b/sysdeps/unix/sysv/linux/microblaze/sys/procfs.h
index 17c52519e7..7a9832371e 100644
--- a/sysdeps/unix/sysv/linux/microblaze/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/microblaze/sys/procfs.h
@@ -91,8 +91,8 @@  struct elf_prpsinfo
     char pr_zomb;			/* Zombie.  */
     char pr_nice;			/* Nice val.  */
     unsigned long int pr_flag;		/* Flags.  */
-    unsigned short int pr_uid;
-    unsigned short int pr_gid;
+    unsigned int pr_uid;
+    unsigned int pr_gid;
     int pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing.  */
     char pr_fname[16];			/* Filename of executable.  */
diff --git a/sysdeps/unix/sysv/linux/mips/sys/procfs.h b/sysdeps/unix/sysv/linux/mips/sys/procfs.h
index 2be0c7e818..523317dec7 100644
--- a/sysdeps/unix/sysv/linux/mips/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/mips/sys/procfs.h
@@ -97,8 +97,8 @@  struct elf_prpsinfo
 #else
     unsigned long int pr_flag;		/* Flags.  */
 #endif
-    long pr_uid;
-    long pr_gid;
+    unsigned int pr_uid;
+    unsigned int pr_gid;
     int pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
     char pr_fname[16];			/* Filename of executable.  */
diff --git a/sysdeps/unix/sysv/linux/nios2/sys/procfs.h b/sysdeps/unix/sysv/linux/nios2/sys/procfs.h
index a61fe96c6b..1bb18f1e58 100644
--- a/sysdeps/unix/sysv/linux/nios2/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/nios2/sys/procfs.h
@@ -91,8 +91,8 @@  struct elf_prpsinfo
     char pr_zomb;			/* Zombie.  */
     char pr_nice;			/* Nice val.  */
     unsigned long int pr_flag;		/* Flags.  */
-    unsigned short int pr_uid;
-    unsigned short int pr_gid;
+    unsigned int pr_uid;
+    unsigned int pr_gid;
     int pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
     char pr_fname[16];			/* Filename of executable.  */
diff --git a/sysdeps/unix/sysv/linux/riscv/sys/procfs.h b/sysdeps/unix/sysv/linux/riscv/sys/procfs.h
index 518de56741..3abbecf957 100644
--- a/sysdeps/unix/sysv/linux/riscv/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/riscv/sys/procfs.h
@@ -82,8 +82,8 @@  struct elf_prpsinfo
     char pr_zomb;			/* Zombie.  */
     char pr_nice;			/* Nice val.  */
     unsigned long int pr_flag;		/* Flags.  */
-    long int pr_uid;
-    long int pr_gid;
+    unsigned int pr_uid;
+    unsigned int pr_gid;
     int pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
     char pr_fname[16];			/* Filename of executable.  */
diff --git a/sysdeps/unix/sysv/linux/sys/procfs.h b/sysdeps/unix/sysv/linux/sys/procfs.h
index 3c417bab29..6af44c02ca 100644
--- a/sysdeps/unix/sysv/linux/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/sys/procfs.h
@@ -86,8 +86,8 @@  struct elf_prpsinfo
     char pr_zomb;			/* Zombie.  */
     char pr_nice;			/* Nice val.  */
     unsigned long int pr_flag;		/* Flags.  */
-    unsigned short int pr_uid;
-    unsigned short int pr_gid;
+    unsigned int pr_uid;
+    unsigned int pr_gid;
     int pr_pid, pr_ppid, pr_pgrp, pr_sid;
     /* Lots missing */
     char pr_fname[16];			/* Filename of executable.  */