Bug 188881
| Summary: | Function dcbnl_cee_fill() may return improper values on failures | ||
|---|---|---|---|
| Product: | Networking | Reporter: | bianpan (bianpan2010) |
| Component: | Other | Assignee: | Stephen Hemminger (stephen) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | linux-4.9-rc6 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
| Attachments: | The patch fixes the bug | ||
Created attachment 256431 [details]
The patch fixes the bug
The patch has been merged into the latest version of the Linux kernel. So I will close the bug.
|
In function dcbnl_cee_fill() defined in file net/dcb/dcbnl.c, variable err takes the error code. Variable err is initialized with "-EMSGSIZE", but it may be reset at lines 1235, 1241, 1274, etc. And the value of err may be 0 on subsequent failures (see lines 1250, 1263, 1271, etc.). That is, function dcbnl_cee_fill() may return 0 (indicates success) even when there are errors. A simple solution is inserting statement "err = -EMSGSIZE;" before the return statement "return err;" at line 1356. Codes are summarised as follows. dcbnl_cee_fill @@ net/dcb/dcbnl.c 1219 static int dcbnl_cee_fill(struct sk_buff *skb, struct net_device *netdev) 1220 { 1221 struct nlattr *cee, *app; 1222 struct dcb_app_type *itr; 1223 const struct dcbnl_rtnl_ops *ops = netdev->dcbnl_ops; 1224 int dcbx, i, err = -EMSGSIZE; 1225 u8 value; 1226 1227 if (nla_put_string(skb, DCB_ATTR_IFNAME, netdev->name)) 1228 goto nla_put_failure; ... 1233 /* local pg */ 1234 if (ops->getpgtccfgtx && ops->getpgbwgcfgtx) { // variable err is reset 1235 err = dcbnl_cee_pg_fill(skb, netdev, 1); 1236 if (err) 1237 goto nla_put_failure; 1238 } 1239 1240 if (ops->getpgtccfgrx && ops->getpgbwgcfgrx) { 1241 err = dcbnl_cee_pg_fill(skb, netdev, 0); 1242 if (err) 1243 goto nla_put_failure; 1244 } 1245 1246 /* local pfc */ 1247 if (ops->getpfccfg) { 1248 struct nlattr *pfc_nest = nla_nest_start(skb, DCB_ATTR_CEE_PFC); 1249 // the value of err may be 0 even if the test fails 1250 if (!pfc_nest) 1251 goto nla_put_failure; 1252 1253 for (i = DCB_PFC_UP_ATTR_0; i <= DCB_PFC_UP_ATTR_7; i++) { 1254 ops->getpfccfg(netdev, i - DCB_PFC_UP_ATTR_0, &value); 1255 if (nla_put_u8(skb, i, value)) 1256 goto nla_put_failure; 1257 } 1258 nla_nest_end(skb, pfc_nest); 1259 } ... 1345 /* DCBX state */ 1346 if (dcbx >= 0) { 1347 err = nla_put_u8(skb, DCB_ATTR_DCBX, dcbx); 1348 if (err) 1349 goto nla_put_failure; 1350 } 1351 return 0; 1352 1353 dcb_unlock: 1354 spin_unlock_bh(&dcb_lock); 1355 nla_put_failure: // insert "err = -EMSGSIZE;" here? 1356 return err; 1357 } Thanks very much!