Bug 188881 - Function dcbnl_cee_fill() may return improper values on failures
Summary: Function dcbnl_cee_fill() may return improper values on failures
Status: RESOLVED CODE_FIX
Alias: None
Product: Networking
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Stephen Hemminger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:05 UTC by bianpan
Modified: 2017-05-12 00:02 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (931 bytes, patch)
2017-05-12 00:01 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 11:05:59 UTC
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!
Comment 1 bianpan 2017-05-12 00:01:58 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.