Fixes have been proposed for this problem at least twice before. ================
(These messages are not presented as a thread so I've put links to each of them) Problem report: <https://lkml.org/lkml/2013/12/15/59> Patch proposal: <https://lkml.org/lkml/2013/12/15/60> Reply suggesting improved presentation: <https://lkml.org/lkml/2013/12/15/62> Revised patch proposal: <https://lkml.org/lkml/2013/12/15/96> Duplicate revised patch proposal: <https://lkml.org/lkml/2013/12/15/97> Doron Tsur proposed: - (nlh)->nlmsg_len <= (len)) + (int)(nlh)->nlmsg_len <= (len)) This would correctly function as long as nlmsg_length were <= INT_MAX. I imagine that this would always be the case since Linux isn't used on machines with ints narrower than 32 bits. It would cause a GCC warning on 32-bit machines when len has an unsigned type that is the same width as int. Programs conforming the the netlink documentation would not get warnings. There was no reply to the revised patch proposal. ================ <https://lkml.org/lkml/2015/3/5/38> Mike Frysinger proposed: -#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \ +#define NLMSG_OK(nlh,len) ((len) >= sizeof(struct nlmsghdr) && \ This should correctly function as long as len is an unsigned type or it has a non-negative value. It won't work correctly if len is size_t and has the value -1 (the error indicator). David Miller replied: I don't think we can change this. If you get rid of the 'int' cast then code is going to end up with a signed comparison for the first test even if 'len' is signed, and that's a potential security issue. I don't understand this response. If you get rid of the int cast, and the type of len, after the "integral promotions", is the same width as size_t, the "usual arithmetic conversions" of the C language will cause the comparison to be done in unsigned. I would agree with a variant of the reply: I don't think we can change this. If you get rid of the 'int' cast then IN MOST ENVIRONMENTS the code is going to end up with an UNSIGNED comparison for the first test even if 'len' is signed, and that's a potential security issue. Consider the case where len is a signed type with a negative value, such as -1, the error indicator described in recv(2). The example code in netlink(7) about reading netlink message is a good example of code that would misbehave if the cast were removed. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
