illumos gate

Issue¶

The dlmgmt_door_setattr_t struct is defined as

typedef struct dlmgmt_door_setattr_s {
    int            ld_cmd;
    int            ld_confid;
    char            ld_attr[MAXLINKATTRLEN];
    size_t            ld_attrsz;
    dladm_datatype_t    ld_type;
    char            ld_attrval[MAXLINKATTRVALLEN];
} dlmgmt_door_setattr_t;

This struct is passed from libdladm to dlmgmtd through a door. dlmgmt is a 32-bit daemon process. If the calling process using libdladm is 64-bits this means ld_attrsz will be sent as an 8 byte value and interpreted as a 4 byte value. There is a precondition check for the door handler that will reject any 64 bit callers due to a size mismatch.

    infop = dlmgmt_getcmdinfo(door_arg->ld_cmd);
    if (infop == NULL || argsz != infop->di_reqsz) {
        err = EINVAL;
        goto done;
    }

Here infop->di_reqsz comes from the following

    { DLMGMT_CMD_SETATTR, sizeof (dlmgmt_door_setattr_t),
        sizeof (dlmgmt_setattr_retval_t), dlmgmt_setattr }

meaning any 64-bit caller will be off by 4 bytes and get an immediate EINVAL.

Possible Solution¶

I think changing the size_t to a uint32_t is a viable solution here.

ry@ryz:~/pr/illumos-gate$ git diff
diff --git a/usr/src/lib/libdladm/common/libdlmgmt.h b/usr/src/lib/libdladm/common/libdlmgmt.h
index 2e208209e8..8ea0f6650f 100644
--- a/usr/src/lib/libdladm/common/libdlmgmt.h
+++ b/usr/src/lib/libdladm/common/libdlmgmt.h
@@ -98,7 +98,7 @@ typedef struct dlmgmt_door_setattr_s {
        int                     ld_cmd;
        int                     ld_confid;
        char                    ld_attr[MAXLINKATTRLEN];
-       size_t                  ld_attrsz;
+       uint32_t                ld_attrsz;
        dladm_datatype_t        ld_type;
        char                    ld_attrval[MAXLINKATTRVALLEN];
 } dlmgmt_door_setattr_t;

I'm happy to send this to the illumos advocates mailing list if folks think this is a reasonable solution.

Imact on other systems¶

Direct usage of the ld_attrsz attribute appears to be limited to the dlmgmt_door.c code referenced above and the code in libdlmgmt.c that sets this property before a door call via dladm_set_conf_field.

ry@ryz:~/pr/illumos-gate/usr/src$ find . -name "*" -exec grep -n ld_attrsz {} +
./lib/libdladm/common/libdlmgmt.c:661:  setattr.ld_attrsz = attrsz;
./lib/libdladm/common/libdlmgmt.h:101:  uint32_t                ld_attrsz;
./cmd/dlmgmtd/dlmgmt_door.c:782:            &setattr->ld_attrval, setattr->ld_attrsz, setattr->ld_type);

similarly for references to the overall struct dlmgmt_door_setattr_t

ry@ryz:~/pr/illumos-gate/usr/src$ find . -name "*" -exec grep -n dlmgmt_door_setattr_t {} +
./lib/libdladm/common/libdlmgmt.c:639:  dlmgmt_door_setattr_t   setattr;
./lib/libdladm/common/libdlmgmt.h:104:} dlmgmt_door_setattr_t;
./cmd/dlmgmtd/dlmgmt_door.c:761:        dlmgmt_door_setattr_t   *setattr = argp;
./cmd/dlmgmtd/dlmgmt_door.c:1356:       { DLMGMT_CMD_SETATTR, sizeof (dlmgmt_door_setattr_t),

Testing¶

I have been running the above patch on my system which does allow me to call libdladm functions that call out to dlmgmt_setattr with no EINVAL coming back from the subsequent door call, and the desired results of the called function are observed.