net: Validate interface group names in ioctl handlers

[markj]

Summary

The handlers were not checking that the group names are nul-terminated.
Add checks for this.

Reported by: Ilja Van Sprundel <[e-mail]>

[zlei]

sys/net/if.c

2853

What about the error ENAMETOOLONG ? I think that is more precise what's wrong. Just like error of the renaming of interface,

# ifconfig epair0a name aabbccddaabbccdd
ifconfig: ioctl SIOCSIFNAME (set name): File name too long

[ivy]

sys/net/if.c

2853

ENAMETOOLONG does not seem appropriate, even if it's used elsewhere:

63 ENAMETOOLONG File name too long. A component of a path name exceeded
    {NAME_MAX} characters, or an entire path name exceeded {PATH_MAX}
    characters.  See also the description of _PC_NO_TRUNC in pathconf(2).

however, wouldn't this be an ideal place to use exterr?

[glebius]

sys/net/if.c

2853

IMHO, ENAMETOOLONG is better than EINVAL, although mentions a file. Of course a proper exterrorizing is better. But if there is no extra time to work on that, I would prefer ENAMETOOLONG.

[zlei]

sys/net/if.c

2853

The fact is that we have limited errors to use, and some, EINVAL e.g., are too generic to hint what's wrong behind.

So if we can use a more precise error, and do not lead to much confusion, then that is appropriate.

Fo example, use error ENAMETOOLONG instead of EINVAL in this context, that rename interfaces or adding / removing a group, although the error ENAMETOOLONG is from the filesystem domain, but it should not cause confusion in the network domain, hence IMO it is an appropriate usage.

[markj]

sys/net/if.c

2853

The error is because the group name buffer is not nul-terminated, not because the name is too long. It's up to userspace to check whether the group name is too long. So I think ENAMETOOLONG is wrong.

I don't have any plan to exterrize this. The group ioctls themselves are simple and probably don't need it (most of the error paths are from if_addgroup() incorrectly using M_NOWAIT to perform allocations), though I agree that it'd be useful for some other ifnet ioctls.

[zlei]

sys/net/if.c

2853

I see your point. That the userland should pass in a struct ifgroupreq in good shape. Then EINVAL is proper.