> > > static int igbvf_change_mtu(struct net_device *netdev, int new_mtu) > > > { > > > struct igbvf_adapter *adapter = netdev_priv(netdev); > > > - int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN; > > > + unsigned int max_frame = new_mtu + ETH_HLEN + ETH_FCS_LEN; > > > > > > if ((new_mtu < 68) || (max_frame > MAX_JUMBO_FRAME_SIZE)) { > > > dev_err(&adapter->pdev->dev, "Invalid MTU setting\n"); > > > > It is safer to check: > > if ((new_mtu < 68) || (new_mtu > MAX_JUMBO_FRAME_SIZE - ETH_HLEN - ETH_FCS_LEN)) { > > > > I believe my fix is already 100% safe... Where is the bug in my code? > > Your fix harder to read because of the additional math and because it's > checking "new_mtu" when we care about "max_frame". I don't like the window check on two variables. However if ETH_HLEN and ETH_FCS_LEN are 'int' (not an unsigned type) and 'new_mtu' is just below MAX_INT then the signed arithmetic could overflow - generating an indeterminate value. In which case 'max_frame' might not exceed MAX_JUMBO_FRAME_SIZE. David -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html