Project

General

Profile

Actions

Bug #13882

closed

libipadm ipadm_if_info() is not 64bit safe

Added by Hans Rosenfeld almost 2 years ago. Updated almost 2 years ago.

Status:
Closed
Priority:
Normal
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

The type checks I added to #13863 showed that ipadm_if_info() may suffer from a similar issue as #13866.

The underlying door call uses ipmgmt_getif_rval_t, which embeds an array of ipadm_if_info_t structures, which in turn contains a pointer used for making a single-linked list of ipadm_if_info_t structures:

typedef struct ipadm_if_info_s {
    struct ipadm_if_info_s    *ifi_next;
    char            ifi_name[LIFNAMSIZ];    /* interface name */
    ipadm_if_state_t    ifi_state;        /* see above */
    uint_t            ifi_cflags;        /* current flags */
    uint_t            ifi_pflags;        /* persistent flags */
} ipadm_if_info_t;

The door call from libipadm to ipmgmtd doesn't use the ifi_next pointer, it just copies an array across the door and sets the list pointers later. Due to the different pointer size the offset of ifi_name is wrong, though.

A solution to that problem is to remove ifi_next from ipadm_if_info_t and add use an extra container structure ipadm_if_info_list_t. This will break the libipadm API and requires a bit of rework in the code using it, but at least it's private to illumos-gate.

Actions #1

Updated by Electric Monk almost 2 years ago

  • Gerrit CR set to 1551
Actions #2

Updated by Hans Rosenfeld almost 2 years ago

  • Description updated (diff)
Actions #3

Updated by Hans Rosenfeld almost 2 years ago

  • Description updated (diff)
Actions #4

Updated by Hans Rosenfeld almost 2 years ago

Testing: I built both a 32bit and 64bit libipadm and verified communication with a 32bit ipmgmtd works for both. I also verified that "ipadm show-if" still works in the global zone as well as a non-global zone. I also checked "ifconfig -a", and the output looked as expected.

Actions #5

Updated by Electric Monk almost 2 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ed1e93792d7c9ea04a0cb44cffe34c24c135b002

commit  ed1e93792d7c9ea04a0cb44cffe34c24c135b002
Author: Hans Rosenfeld <rosenfeld@grumpf.hope-2000.org>
Date:   2021-07-27T16:12:26.000Z

    13882 libipadm ipadm_if_info() is not 64bit safe
    Reviewed by: Jason King <jason.brian.king+illumos@gmail.com>
    Reviewed by: Robert Mustacchi <rm+illumos@gmail.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF