C reading mac address - undefined behavior

Question

I would like to read the MAC address of my target. So I am using the following code

TS32 get_mac_address(TU8 ** aps8Mac)
{
    TS32 fd;
    struct ifreq ifr;
    CHAR *iface = "eth0";
    TU8 *mac;

    fd = socket(AF_INET, SOCK_DGRAM, 0);

    ifr.ifr_addr.sa_family = AF_INET;
    strncpy(ifr.ifr_name , iface , IFNAMSIZ-1);

    ioctl(fd, SIOCGIFHWADDR, &ifr);

    close(fd);

    mac = (unsigned char *)ifr.ifr_hwaddr.sa_data;
    *aps8Mac = mac;

    return 0;
}

int main(int argc, char **argv)
{
    TU8   *s8Mac = NULL;
    get_mac_address(&s8Mac);
    fprintf(stdout, "MAC address : %.2X:%.2X:%.2X:%.2X:%.2X:%.2X\n" , s8Mac[0], s8Mac[1], s8Mac[2], s8Mac[3], s8Mac[4], s8Mac[5]); 
    fprintf(stdout, "MAC address : %.2X:%.2X:%.2X:%.2X:%.2X:%.2X\n" , s8Mac[0], s8Mac[1], s8Mac[2], s8Mac[3], s8Mac[4], s8Mac[5]);
    return 0;
}

Once executed on the target I am getting :

MAC address : 00:01:02:03:00:76
MAC address : FE:76:9C:8C:AB:7E

Why I do not read

MAC address : 00:01:02:03:00:76
MAC address : 00:01:02:03:00:76

EDIT 1 : get_mac_address modified regarding to advices given as answer

TS32 get_mac_address(TU8 ** aps8Mac)
{
    TS32 fd;
    struct ifreq ifr;
    CHAR *iface = "eth0";

    TU8 *mac = malloc(sizeof(TU8) * 17);

    fd = socket(AF_INET, SOCK_DGRAM, 0);

    ifr.ifr_addr.sa_family = AF_INET;
    strncpy(ifr.ifr_name , iface , IFNAMSIZ-1);

  ioctl(fd, SIOCGIFHWADDR, &ifr);

    close(fd);

    mac = (TU8 *)ifr.ifr_hwaddr.sa_data;
    *aps8Mac = mac;
    return 0;
}

@DavidSchwartz I've modified the topic in order to include the modified function. However, I still obtain the same behavior — ogs, Mar 10 '16 at 16:14
You throw away the value you got from `malloc` and still return a pointer to a temporary. `mac = (TU8 *)ifr.ifr_hwaddr.sa_data; *aps8Mac = mac;` is just a roundabout way of setting `*aps8Mac` equal to `ifr.ifr_hwaddr.sa_data`. — David Schwartz, Mar 10 '16 at 17:35

score 3 · Answer 1 · answered Mar 10 '16 at 09:50

You are returning a pointer to a local stack variable. This is illegal - once a function returns, you are no longer allowed to access its local variables. The correct approach is to allocate memory in the caller and have the callee copy the result there.

The reason the correct result prints once is that returning from a C function does not zero its stack space. The values happen to still be intact on the stack, so they get passed to fprintf. Then the fprintf call uses the stack to do its work, putting different data in the pointed-to location.

I've modified the question in order to allocate memory in the caller as you adviced, but the behavior is still the same. I guess I've wrongly understood something. — ogs, Mar 10 '16 at 16:17

abelenky · Accepted Answer · 2016-03-18T12:41:54.110

After your edit, you have allocated 17 bytes for your mac address.
(I am unsure why you wanted 17 bytes... a typical MAC address is only 6-bytes long!)

However, you do not fill in that data properly.

You tried to fill it in on line:

mac = (TU8 *)ifr.ifr_hwaddr.sa_data;

But all that does is give the mac pointer the same pointer as sa_data, while simultaneously leaking the memory you allocated before.

I believe you want to copy the data from one structure to another:

// Copy 17 bytes from the sa_data to the allocated, reserved space for mac-address
// After this, ifr-->sa_data can be overwritten or destroyed; we don't care
// We have our own copy of the data in mac.
memcpy(mac, (TU8 *)ifr.ifr_hwaddr.sa_data, 17*sizeof(TU8) );

I would attempt to re-write it like this:

TS32 get_mac_address(TU8 ** aps8Mac)
{
    TS32 fd;
    struct ifreq ifr;
    CHAR *iface = "eth0";
    const int MAC_SIZE = 6; // MAC addresses are 6-bytes.

    *aps8Mac = malloc(sizeof(TU8) * MAC_SIZE);

    fd = socket(AF_INET, SOCK_DGRAM, 0);

    ifr.ifr_addr.sa_family = AF_INET;
    strncpy(ifr.ifr_name , iface , IFNAMSIZ-1);

    ioctl(fd, SIOCGIFHWADDR, &ifr);
    close(fd);

    // Copy the 6-byte address to the output.
    memcpy(*aps8Mac, ifr.ifr_hwaddr.sa_data, MAC_SIZE);

    return 0;
}

Thank for your response I understand now. In your function, It is necessary to specify the MAC_SIZE's type, isn't ? — ogs, Mar 18 '16 at 09:18

C reading mac address - undefined behavior

2 Answers2