Bug 301 - parse.c makes incorrect assumption about size of macros
parse.c makes incorrect assumption about size of macros
Status: RESOLVED FIXED
Product: Sudo
Classification: Unclassified
Component: Sudo
1.7.0
IBM AIX
: low normal
Assigned To: Todd C. Miller
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-29 02:06 MDT by Dale King
Modified: 2008-10-03 10:03 MDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dale King 2008-09-29 02:06:24 MDT
Hi,

Compiling sudo under AIX I was trying to work out why:
  Defaults !noexec

would not be parsed correctly and more importantly why noexec is always enabled by default.

I am using the IBM Visual Age C V6 on AIX 5.3.  GCC 4.2 exhibits the same behaviour.

Line 297 of parse.c has this test:
    if (tags->noexec!= UNSPEC)

which fails due to the wrong assumption about the size of:
    #define UNSPEC -1

which is a 32bit signed integer.  Digging down, I have the following situation:

    sizeof char = 1
    sizeof UNSPEC = 4
    sizeof tags->noexec = 1

    tags->noexec = ff
    UNSPEC = ffffffff

A workaround is to change the value of UNSPEC from -1 to 3, which fixes the problem for me.
Comment 1 Todd C. Miller 2008-10-01 06:48:34 MDT
Actually, this is a sign-related issue.  On the Power architecture (and unlike most other cpu architectures), char is unsigned by default.  This means that when a char is compared to -1, the -1 is promoted to unsigned int and the comparison fails because 0xff != 0xffffffff.

The simplest fix is to just make the contents of struct cmndtag be "signed char", which is what I will probably do.  I worry that there is code that assumes UNSPEC <= FALSE (there used to be), so changing the type seems safest.
Comment 2 Dale King 2008-10-01 20:07:49 MDT
(In reply to comment #1)

> The simplest fix is to just make the contents of struct cmndtag be
> "signed char", which is what I will probably do.  I worry that there is
> code that assumes UNSPEC <= FALSE (there used to be), so changing the
> type seems safest.


I tested changing the contents of cmndtag to signed types today and it works fine.  From a quick grep through the headers I think this is the only place that needs fixing.
Comment 3 Todd C. Miller 2008-10-03 10:03:45 MDT
I've added configure tests for "__signed" and "signed" and adjusted parse.h accordingly.