[syslinux] EXTLINUX - GCC 5

Thomas Schmitt scdbackup at gmx.net
Sat Jul 11 03:11:48 PDT 2015


Hi,

Gene Cumm wrote:
> > 3) It feels like this is a moving target where gcc keeps changing and
> > different results get reported.

Do we have indications that different versions of gcc5
cause different behavior on the same build and boot machines ?


Ady wrote:
> Since the issue is only present on specific 
> hardware / firmware, whatever might seem to "solve" the behavior in one 
> system might not be really a solution for others.

One should at least determine whether the problem is with
the machines which build SYSLINUX or with those which try
to boot it.
(E.g. is a ISO with gcc5-built ISOLINUX bootable on some
 machines and not on others ?
 Does one machine build good ISOs with gcc5 and the other
 produces bad ones ?)


My best theory is still that SYSLINUX would have some C code
parts which allow the compilers to produce non-equivalent
machine code.

I see complaints in the output of cppcheck 1.67, which could
match this criterium.
(Maybe one should subscribe SYSLINUX to Coverty Scan ?)

-----------------------------------------------------------------

  [com32/gpllib/dmi/dmi_memory.c:205]: (error) Undefined behavior: Variable 'type' is used as parameter and destination in s[n]printf().
  [com32/gpllib/dmi/dmi_memory.c:217]: (error) Undefined behavior: Variable 'connection' is used as parameter and destination in s[n]printf().
  [com32/gpllib/dmi/dmi_memory.c:247]: (error) Undefined behavior: Variable 'size' is used as parameter and destination in s[n]printf().
  [com32/gpllib/dmi/dmi_memory.c:249]: (error) Undefined behavior: Variable 'size' is used as parameter and destination in s[n]printf().

The complaint says it all: It is an invitation for trouble to
use the same memory as destination and source of a copy
operation.

-----------------------------------------------------------------

  [com32/gpllib/dmi/dmi.c:322] -> [com32/gpllib/dmi/dmi.c:321]: (warning) Array 'type[13]' accessed at index 13, which is out of bounds. Otherwise condition 'code<=13' is redundant.

is caused by missing subtraction of 1 in

        strlcpy(dmi->base_board.type, type[code],

It should be

        strlcpy(dmi->base_board.type, type[code - 1],


-----------------------------------------------------------------

  [com32/gpllib/dmi/dmi_processor.c:381] -> [com32/gpllib/dmi/dmi_processor.c:380]: (warning) Array 'upgrade[23]' accessed at index 24, which is out of bounds. Otherwise condition 'code<=25' is redundant.

is caused by missing commas in lines

        "Socket 939"            /* 0x12 */
        "Socket LGA1366"        /* 0x19 */

-----------------------------------------------------------------

  [com32/hdt/hdt-cli.c:609]: (error) Possible null pointer dereference: argv

Hard to say whether this is really wrong code.
It depends on the two while-loops in parse_command_line() whether
the second allocates as many items as the the first one predicted.

It is very optimistic about success of malloc().

-----------------------------------------------------------------

  [com32/hdt/hdt-cli.c:90]: (error) Memory is allocated but not initialized: new

Might be a false positive.

-----------------------------------------------------------------

  [com32/lib/jpeg/tinyjpeg.c:314] -> [com32/lib/jpeg/tinyjpeg.c:309]: (warning) Array 'DCT[64]' accessed at index 64, which is out of bounds. Otherwise condition 'j>=64' is redundant.

Probably a false positive.
(What is the reason of macro "__unlikely()" which is defined as "(!!(x))" ?)

-----------------------------------------------------------------

  [com32/lib/syslinux/pxe_dns.c:56]: (error) Uninitialized variable: q

False positive.
cppcheck seems to not understand the use of the return value of sscanf().

-----------------------------------------------------------------

  [com32/libupload/upload_tftp.c:138]: (error) Uninitialized struct member: tftp.srv_ip

Looks like a valid complaint.
But from where to get a valid tftp.srv_ip ?

-----------------------------------------------------------------

  [com32/modules/prdhcp.c:145]: (error) Uninitialized variable: p

Possibly a false positive.
It depends on what pxe_get_cached_info() does to &p.

-----------------------------------------------------------------

  Checking com32/rosh/rosh.c: COMMAND_LINE_SIZE...
  [com32/rosh/rosh.c:1128]: (error) Uninitialized variable: cmdstr

  Checking com32/rosh/rosh.c: FILENAME_MAX...
  [com32/rosh/rosh.c:559]: (error) Uninitialized variable: filestr2
  ...
  [com32/rosh/rosh.c:600]: (error) Uninitialized variable: filestr2

Possibly non-functional compile time alternatives due to
unqualified macro settings by cppcheck. 

-----------------------------------------------------------------

  [com32/sysdump/memmap.c:50]: (error) Possible null pointer dereference: buf
  [com32/sysdump/memmap.c:51]: (error) Possible null pointer dereference: buf
  [com32/sysdump/memmap.c:52]: (error) Possible null pointer dereference: buf

Probably false positive.
cppcheck has problems to recognize memory allocation on demand.
 
-----------------------------------------------------------------

  [core/fs/pxe/dnsresolv.c:107]: (error) Uninitialized variable: ip

Probably false positive.

-----------------------------------------------------------------

  [core/fs/xfs/xfs_dir2.c:588]: (error) Uninitialized variable: irec
  ...
  [core/fs/xfs/xfs_dir2.c:584]: (error) Uninitialized struct member: irec.br_blockcount

Hard to say.
Who understands XFS structure ?

-----------------------------------------------------------------

  Checking core/lwip/src/core/ipv4/igmp.c: LWIP_DEBUG;LWIP_IGMP...
  [core/lwip/src/core/ipv4/igmp.c:813]: (error) Possible null pointer dereference: igmp

Looks like a valid complaint.
Will cause SIGSEGV in case of pbuf_alloc() returning 0.
Question is whether the macro combination is supposed to be
functional.

-----------------------------------------------------------------

  Checking core/lwip/src/core/ipv4/igmp.c: LWIP_DEBUG;LWIP_IGMP...
  [core/lwip/src/core/ipv4/igmp.c:397]: (error) Uninitialized variable: igmp

Hard to say.
Possibly a non-functional macro combination.

-----------------------------------------------------------------

  [core/serirq.c:203]: (error) Null pointer dereference

Looks like confusion of memcpy() and memset():

        memcpy(SerialHead, 0x0, serial_buf_size);

should probably be

        memset(SerialHead, 0x0, serial_buf_size);

-----------------------------------------------------------------

  [gpxe/src/core/iobuf.c:50]: (error) Possible null pointer dereference: iobuf
  ...
  [gpxe/src/core/iobuf.c:51]: (error) Null pointer dereference

Probably a false positive.
cppcheck seems not to understand __alignof__() which does not
dereference *iobuf but just inspects its type.

-----------------------------------------------------------------

  [gpxe/src/core/uri.c:154]: (error) Possible null pointer dereference: authority

Indeed the code path through "else" of

        /* Identify net/absolute/relative path */
        if ( strncmp ( path, "//", 2 ) == 0 ) {

does not set variable authority.

-----------------------------------------------------------------

  [gpxe/src/crypto/axtls/aes.c:251]: (error) Uninitialized variable: t1

Crypto stuff is cryptic by intention.

-----------------------------------------------------------------

  [gpxe/src/drivers/net/e1000/e1000_hw.c:7395]: (error) Uninitialized variable: phy_data
  ...
  [gpxe/src/drivers/net/e1000/e1000_hw.c:7557]: (error) Uninitialized variable: phy_data

Might be a false positive.
The code path which does not set phy_data might be separated from
the code paths which use it.

-----------------------------------------------------------------

  [gpxe/src/util/efirom.c:68]: (error) Uninitialized variable: len

A function which is intended to work by clairvoyance ?

  static size_t file_size ( FILE *file ) {
          ssize_t len;

          return len;
  }

(Its quality as random number generator is mediocre, too.
 There's too much redundancy in stack usage.)

-----------------------------------------------------------------

  Checking gpxe/src/util/elf2efi.c...
  [gpxe/src/util/elf2efi.c:352]: (error) Uninitialized variable: data_start
  Checking gpxe/src/util/elf2efi.c: MDE_CPU_IA32...
  Checking gpxe/src/util/elf2efi.c: MDE_CPU_X64...

Something new: A non-functional code branch if no macro is set.

-----------------------------------------------------------------

  Checking gpxe/src/util/nrv2b.c: ENCODE...
  [gpxe/src/util/nrv2b.c:634]: (error) Array 's.best_pos[1]' accessed at index 2, which is out of bounds.
  [gpxe/src/util/nrv2b.c:635]: (error) Array 's.best_pos[1]' accessed at index 2, which is out of bounds.

Clearly a hardcoded wrong index.

  #define SWD_BEST_OFF    1
  ...
  struct ucl_swd
  {
    ...
  #if defined(SWD_BEST_OFF)
        unsigned int best_pos[ SWD_BEST_OFF ];
  #endif
    ...
  };
  ...
  #if defined(SWD_BEST_OFF)
        if (s->best_pos[2] == 0)
                s->best_pos[2] = key + 1;
  #endif

Shall the index be [SWD_BEST_OFF - 1] ?
Shall this be done only if SWD_BEST_OFF > 2 ?

-----------------------------------------------------------------

  Checking linux/syslinux.c: __KLIBC__...
  [linux/syslinux.c:123]: (error) Uninitialized variable: loop_fd

Valid complaint:

  /*
   * If DO_DIRECT_MOUNT is 0, call mount(8)
   * If DO_DIRECT_MOUNT is 1, call mount(2)
   */
  #ifdef __KLIBC__
  # define DO_DIRECT_MOUNT 1
  #else
  # define DO_DIRECT_MOUNT 0      /* glibc has broken losetup ioctls */
  #endif
  ...
  #if DO_DIRECT_MOUNT
    {
        if (!S_ISBLK(st.st_mode)) {
            /* It's file, need to mount it loopback */
            ...
            int loop_fd;

            for (n = 0; loop_fd < 0; n++) {
            ...
                if (ioctl(loop_fd, LOOP_SET_FD, (void *)dev_fd)) {

losetup ioctls have few chance if they get called by random decision.

This would be a nice suspect if __KLIBC__ is defined.

-----------------------------------------------------------------

  [win/syslinux.c:506]: (error) Uninitialized variable: sdn

Possibly a false positive.

-----------------------------------------------------------------

Have a nice day :)

Thomas



More information about the Syslinux mailing list