diff mbox

elf.h SHF_EXCLUDE signed int 31 bit shift triggers undefined behaviour.

Message ID 1427193579-26102-1-git-send-email-mjw@redhat.com
State Superseded
Headers

Commit Message

Mark Wielaard March 24, 2015, 10:39 a.m. UTC 
  Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
will trigger undefined behaviour because it is defined as a 31 bit shift
against an signed integer. Fix by explicitly using an unsigned int.
---
 ChangeLog | 4 ++++
 elf/elf.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Szabolcs Nagy March 24, 2015, 2:13 p.m. UTC | #1 
On 24/03/15 10:39, Mark Wielaard wrote:
> Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> will trigger undefined behaviour because it is defined as a 31 bit shift
> against an signed integer. Fix by explicitly using an unsigned int.

there is another proposed patch for this

https://sourceware.org/ml/libc-alpha/2015-03/msg00287.html

>  ChangeLog | 4 ++++
>  elf/elf.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 

i think changelog entries are supposed to be submitted separately
Mike Frysinger March 24, 2015, 7:50 p.m. UTC | #2 
On 24 Mar 2015 14:13, Szabolcs Nagy wrote:
> On 24/03/15 10:39, Mark Wielaard wrote:
> > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> > will trigger undefined behaviour because it is defined as a 31 bit shift
> > against an signed integer. Fix by explicitly using an unsigned int.
> 
> there is another proposed patch for this
> 
> https://sourceware.org/ml/libc-alpha/2015-03/msg00287.html
> 
> >  ChangeLog | 4 ++++
> >  elf/elf.h | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> 
> i think changelog entries are supposed to be submitted separately

nope ... should be same commit
-mike
Florian Weimer March 24, 2015, 7:53 p.m. UTC | #3 
* Mike Frysinger:

> On 24 Mar 2015 14:13, Szabolcs Nagy wrote:
>> On 24/03/15 10:39, Mark Wielaard wrote:
>> > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
>> > will trigger undefined behaviour because it is defined as a 31 bit shift
>> > against an signed integer. Fix by explicitly using an unsigned int.
>> 
>> there is another proposed patch for this
>> 
>> https://sourceware.org/ml/libc-alpha/2015-03/msg00287.html
>> 
>> >  ChangeLog | 4 ++++
>> >  elf/elf.h | 2 +-
>> >  2 files changed, 5 insertions(+), 1 deletion(-)
>> > 
>> 
>> i think changelog entries are supposed to be submitted separately
>
> nope ... should be same commit

There used be an expectation that the ChangeLog update was not to be
submitted as part of the patch, but as a separate item in the patch
submission (not as a diff hunk, not with + prefix).

Has this changed?  It would certainly make things simpler for me. :)
Mike Frysinger March 24, 2015, 9:41 p.m. UTC | #4 
On 24 Mar 2015 20:53, Florian Weimer wrote:
> * Mike Frysinger:
> > On 24 Mar 2015 14:13, Szabolcs Nagy wrote:
> >> On 24/03/15 10:39, Mark Wielaard wrote:
> >> > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> >> > will trigger undefined behaviour because it is defined as a 31 bit shift
> >> > against an signed integer. Fix by explicitly using an unsigned int.
> >> 
> >> there is another proposed patch for this
> >> 
> >> https://sourceware.org/ml/libc-alpha/2015-03/msg00287.html
> >> 
> >> >  ChangeLog | 4 ++++
> >> >  elf/elf.h | 2 +-
> >> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >> 
> >> i think changelog entries are supposed to be submitted separately
> >
> > nope ... should be same commit
> 
> There used be an expectation that the ChangeLog update was not to be
> submitted as part of the patch, but as a separate item in the patch
> submission (not as a diff hunk, not with + prefix).
> 
> Has this changed?  It would certainly make things simpler for me. :)

i don't know what the preference is, but i don't mind either
-mike
Ondrej Bilka May 13, 2015, 10:27 a.m. UTC | #5 
On Tue, Mar 24, 2015 at 11:39:39AM +0100, Mark Wielaard wrote:
> Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> will trigger undefined behaviour because it is defined as a 31 bit shift
> against an signed integer. Fix by explicitly using an unsigned int.
> ---
>  ChangeLog | 4 ++++
>  elf/elf.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
I looked that Florian acked this at patchwork thread. Florian, could you also
commit it or do we need more discussion?
Mark Wielaard May 13, 2015, 10:46 a.m. UTC | #6 
On Wed, 2015-05-13 at 12:27 +0200, Ondřej Bílka wrote:
> On Tue, Mar 24, 2015 at 11:39:39AM +0100, Mark Wielaard wrote:
> > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> > will trigger undefined behaviour because it is defined as a 31 bit shift
> > against an signed integer. Fix by explicitly using an unsigned int.
> > ---
> >  ChangeLog | 4 ++++
> >  elf/elf.h | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> I looked that Florian acked this at patchwork thread. Florian, could you also
> commit it or do we need more discussion?

It is already in as commit fb4041, see:
https://sourceware.org/ml/libc-alpha/2015-04/msg00359.html
Ondrej Bilka May 13, 2015, 10:56 a.m. UTC | #7 
On Wed, May 13, 2015 at 12:46:26PM +0200, Mark Wielaard wrote:
> On Wed, 2015-05-13 at 12:27 +0200, Ondřej Bílka wrote:
> > On Tue, Mar 24, 2015 at 11:39:39AM +0100, Mark Wielaard wrote:
> > > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> > > will trigger undefined behaviour because it is defined as a 31 bit shift
> > > against an signed integer. Fix by explicitly using an unsigned int.
> > > ---
> > >  ChangeLog | 4 ++++
> > >  elf/elf.h | 2 +-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > I looked that Florian acked this at patchwork thread. Florian, could you also
> > commit it or do we need more discussion?
> 
> It is already in as commit fb4041, see:
> https://sourceware.org/ml/libc-alpha/2015-04/msg00359.html

thanks. marked that also at patchwork.
Carlos O'Donell May 14, 2015, 12:43 a.m. UTC | #8 
On 05/13/2015 06:56 AM, Ondřej Bílka wrote:
> On Wed, May 13, 2015 at 12:46:26PM +0200, Mark Wielaard wrote:
>> On Wed, 2015-05-13 at 12:27 +0200, Ondřej Bílka wrote:
>>> On Tue, Mar 24, 2015 at 11:39:39AM +0100, Mark Wielaard wrote:
>>>> Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
>>>> will trigger undefined behaviour because it is defined as a 31 bit shift
>>>> against an signed integer. Fix by explicitly using an unsigned int.
>>>> ---
>>>>  ChangeLog | 4 ++++
>>>>  elf/elf.h | 2 +-
>>>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>>>
>>> I looked that Florian acked this at patchwork thread. Florian, could you also
>>> commit it or do we need more discussion?
>>
>> It is already in as commit fb4041, see:
>> https://sourceware.org/ml/libc-alpha/2015-04/msg00359.html
> 
> thanks. marked that also at patchwork.
> 

Ondrej,

Thanks for the patchwork cleanup!

c.
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index b0a17b0..bdf899f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@ 
+2015-03-24  Mark Wielaard  <mjw@redhat.com>
+
+	* elf/elf.h (SHF_EXCLUDE): Use unsigned 1 for shift.
+
 2015-03-23  Roland McGrath  <roland@hack.frob.com>
 
 	* libio/iofdopen.c: Move FD_FLAGS declaration into its first use,
diff --git a/elf/elf.h b/elf/elf.h
index 496f08d..960a3c3 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -371,7 +371,7 @@  typedef struct
 #define SHF_MASKPROC	     0xf0000000	/* Processor-specific */
 #define SHF_ORDERED	     (1 << 30)	/* Special ordering requirement
 					   (Solaris).  */
-#define SHF_EXCLUDE	     (1 << 31)	/* Section is excluded unless
+#define SHF_EXCLUDE	     (1U << 31)	/* Section is excluded unless
 					   referenced or allocated (Solaris).*/
 
 /* Section group handling.  */