Re: [PATCH net] bpf: uninitialized variables in test code
I'm afraid Roman's patch doesn't fix the bug.
On Fri, Nov 30, 2018 at 02:58:03PM -0800, Alexei Starovoitov wrote:
> On Thu, Nov 29, 2018 at 01:27:03PM +0300, Dan Carpenter wrote:
> > Smatch complains that if bpf_test_run() fails with -ENOMEM at the
> > begining then the "duration" is uninitialized. We then copy the
> > unintialized variables to the user inside the bpf_test_finish()
> > function. The functions require CAP_SYS_ADMIN so it's not really an
> > information leak.
> >
> > Fixes: 1cf1cae963c2 ("bpf: introduce BPF_PROG_TEST_RUN command")
> > Signed-off-by: Dan Carpenter
>
> That is incorrect fixes tag.
Yeah. You're right. The Fixes tag is wrong. I spent some time looking
at this too, because the code is old but the warning only just
appeared... :/
Thanks for fixing this, Roman.
regards,
dan carpenter
[PATCH net] bpf: uninitialized variables in test code
Smatch complains that if bpf_test_run() fails with -ENOMEM at the
begining then the "duration" is uninitialized. We then copy the
unintialized variables to the user inside the bpf_test_finish()
function. The functions require CAP_SYS_ADMIN so it's not really an
information leak.
Fixes: 1cf1cae963c2 ("bpf: introduce BPF_PROG_TEST_RUN command")
Signed-off-by: Dan Carpenter
---
net/bpf/test_run.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index c89c22c49015..49304192a031 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -114,7 +114,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
union bpf_attr *kattr,
bool is_l2 = false, is_direct_pkt_access = false;
u32 size = kattr->test.data_size_in;
u32 repeat = kattr->test.repeat;
- u32 retval, duration;
+ u32 retval, duration = 0;
int hh_len = ETH_HLEN;
struct sk_buff *skb;
struct sock *sk;
@@ -196,7 +196,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
union bpf_attr *kattr,
u32 repeat = kattr->test.repeat;
struct netdev_rx_queue *rxqueue;
struct xdp_buff xdp = {};
- u32 retval, duration;
+ u32 retval, duration = 0;
void *data;
int ret;
--
2.11.0
[bug report] PCI: Remove NULL device handling from PCI DMA API
Hello Christoph Hellwig,
The patch 4167b2ad5182: "PCI: Remove NULL device handling from PCI
DMA API" from Jan 10, 2018, leads to the following static checker
warning:
drivers/net/ethernet/amd/pcnet32.c:1921 pcnet32_probe1()
warn: variable dereferenced before check 'pdev' (see line 1843)
drivers/net/ethernet/amd/pcnet32.c
1839
1840 dev->base_addr = ioaddr;
1841 lp = netdev_priv(dev);
1842 /* pci_alloc_consistent returns page-aligned memory, so we do
not have to check the alignment */
1843 lp->init_block = pci_alloc_consistent(pdev,
sizeof(*lp->init_block),
This function is called with a NULL "pdev" when we're probing from
pcnet32_probe_vlbus().
1844>init_dma_addr);
1845 if (!lp->init_block) {
1846 if (pcnet32_debug & NETIF_MSG_PROBE)
1847 pr_err("Consistent memory allocation failed\n");
1848 ret = -ENOMEM;
1849 goto err_free_netdev;
1850 }
1851 lp->pci_dev = pdev;
1852
1853 lp->dev = dev;
1854
regards,
dan carpenter
[PATCH net-next] octeontx2-af: Copy the right amount of memory
This is a copy and paste bug where we copied the sizeof() from the chunk
before. We're copying more data than intended but the destination is a
union so it doesn't cause memory corruption.
Fixes: ffb0abd7e9cb ("octeontx2-af: NIX AQ instruction enqueue support")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 8890c95831ca..a4eac3b9ee72 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -573,7 +573,7 @@ static int rvu_nix_aq_enq_inst(struct rvu *rvu, struct
nix_aq_enq_req *req,
sizeof(struct nix_cq_ctx_s));
else if (req->ctype == NIX_AQ_CTYPE_RSS)
memcpy(>rss, ctx,
- sizeof(struct nix_cq_ctx_s));
+ sizeof(struct nix_rsse_s));
else if (req->ctype == NIX_AQ_CTYPE_MCE)
memcpy(>mce, ctx,
sizeof(struct nix_rx_mce_s));
--
2.11.0
Re: [PATCH] net/mlx5: allocate enough space in
On Mon, Oct 22, 2018 at 09:18:43AM +0300, Or Gerlitz wrote:
> On Mon, Oct 22, 2018 at 8:23 AM Dan Carpenter
> wrote:
> >
> > On Sun, Oct 21, 2018 at 01:56:26PM +0300, Or Gerlitz wrote:
> > > I will re-post your patch, this time to netdev since the original
> > > commit is there
> > > and so should be the fix, thanks for reporting/fixing!
> >
> > I didn't realize it had been posted to netdev already so I deliberately
> > left that off the CC. If Dave hasn't applied the original (he probably
>
> you didn't post it to netdev, I don't see how he could have got it
>
I meant commit 328edb499f99 ("net/mlx5: Split FDB fast path prio to
multiple namespaces"). It turns out that Dave has applied that, but I
was expecting to see his S-o-B on it.
It's hard for me to know which tree patches are applied to.
Also I see that I screwed up the subject. Thanks for fixing that.
regards,
dan carpenter
Re: [PATCH] net/mlx5: allocate enough space in
On Sun, Oct 21, 2018 at 01:56:26PM +0300, Or Gerlitz wrote: > I will re-post your patch, this time to netdev since the original > commit is there > and so should be the fix, thanks for reporting/fixing! I didn't realize it had been posted to netdev already so I deliberately left that off the CC. If Dave hasn't applied the original (he probably has now because he is so quick) then it's fine by me if you fold them together. regards, dan carpenter
[PATCH net] qlcnic: fix a return in qlcnic_dcb_get_capability()
These functions are supposed to return one on failure and zero on
success. Returning a zero here could cause uninitialized variable
bugs in several of the callers. For example:
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c:1660 get_iscsi_dcb_priority()
error: uninitialized symbol 'caps'.
Fixes: 48365e485275 ("qlcnic: dcb: Add support for CEE Netlink interface.")
Signed-off-by: Dan Carpenter
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
index 4b76c69fe86d..834208e55f7b 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_dcb.c
@@ -883,7 +883,7 @@ static u8 qlcnic_dcb_get_capability(struct net_device
*netdev, int capid,
struct qlcnic_adapter *adapter = netdev_priv(netdev);
if (!test_bit(QLCNIC_DCB_STATE, >dcb->state))
- return 0;
+ return 1;
switch (capid) {
case DCB_CAP_ATTR_PG:
--
2.11.0
[PATCH net-next] bnxt_en: Copy and paste bug in extended tx_stats
The struct type was copied from the line before but it should be "tx"
instead of "rx". I have reviewed the code and I can't immediately see
that this bug causes a runtime issue.
Fixes: 36e53349b60b ("bnxt_en: Add additional extended port statistics.")
Signed-off-by: Dan Carpenter
---
This is from static analysis and I don't have a way to test it.
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 0fe57e36912b..498b373c992d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1471,7 +1471,7 @@ struct bnxt {
struct rx_port_stats*hw_rx_port_stats;
struct tx_port_stats*hw_tx_port_stats;
struct rx_port_stats_ext*hw_rx_port_stats_ext;
- struct rx_port_stats_ext*hw_tx_port_stats_ext;
+ struct tx_port_stats_ext*hw_tx_port_stats_ext;
dma_addr_t hw_rx_port_stats_map;
dma_addr_t hw_tx_port_stats_map;
dma_addr_t hw_rx_port_stats_ext_map;
--
2.11.0
Re: [PATCH net-next] rxrpc: Remove set but not used variable 'ioc'
On Tue, Oct 09, 2018 at 11:13:16AM +0100, David Howells wrote:
> YueHaibing wrote:
>
> > net/rxrpc/output.c: In function 'rxrpc_reject_packets':
> > net/rxrpc/output.c:527:11: warning:
> > variable 'ioc' set but not used [-Wunused-but-set-variable]
> >
> > It never used since introduction in
>
> I wonder why my compiler doesn't show this warning.
>
> Anyway, NAK: just removing the variable is the wrong fix - you need to look at
> the code more closely. The actual fix is to pass it to kernel_sendmsg()
> instead of 2.
>
> But thanks anyway! Do you want to respin your patch?
>
> > commit ece64fec164f ("rxrpc: Emit BUSY packets when supposed to rather than
> > ABORTs")
>
> Btw, this should be a 'Fixes: ("subject")' line and the patch needs
> to go to net, not net-next.
I told him to do that because it wasn't a bugfix... Probably just Fixes
is the right thing to use though.
regards,
dan carpenter
[PATCH] 9p: potential NULL dereference
p9_tag_alloc() is supposed to return error pointers, but we accidentally
return a NULL here. It would cause a NULL dereference in the caller.
Fixes: 996d5b4db4b1 ("9p: Use a slab for allocating requests")
Signed-off-by: Dan Carpenter
diff --git a/net/9p/client.c b/net/9p/client.c
index 47fa6158a75a..5f23e18eecc0 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -281,7 +281,7 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int
max_size)
int tag;
if (!req)
- return NULL;
+ return ERR_PTR(-ENOMEM);
if (p9_fcall_init(c, >tc, alloc_msize))
goto free_req;
[PATCH net] net: sched: act_ipt: check for underflow in __tcf_ipt_init()
If "td->u.target_size" is larger than sizeof(struct xt_entry_target) we
return -EINVAL. But we don't check whether it's smaller than
sizeof(struct xt_entry_target) and that could lead to an out of bounds
read.
Fixes: 7ba699c604ab ("[NET_SCHED]: Convert actions from rtnetlink to new
netlink API")
Signed-off-by: Dan Carpenter
---
I haven't tested this. Please review carefully.
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 1efbfb10b1fc..8af6c11d2482 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -135,7 +135,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id,
struct nlattr *nla,
}
td = (struct xt_entry_target *)nla_data(tb[TCA_IPT_TARG]);
- if (nla_len(tb[TCA_IPT_TARG]) < td->u.target_size) {
+ if (nla_len(tb[TCA_IPT_TARG]) != td->u.target_size) {
if (exists)
tcf_idr_release(*a, bind);
else
[PATCH net] devlink: double free in devlink_resource_fill()
Smatch reports that devlink_dpipe_send_and_alloc_skb() frees the skb
on error so this is a double free. We fixed a bunch of these bugs in
commit 7fe4d6dcbcb4 ("devlink: Remove redundant free on error path") but
we accidentally overlooked this one.
Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction")
Signed-off-by: Dan Carpenter
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 65fc366a78a4..8c0ed225e280 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2592,7 +2592,7 @@ static int devlink_resource_fill(struct genl_info *info,
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(, info);
if (err)
- goto err_skb_send_alloc;
+ return err;
goto send_done;
}
return genlmsg_reply(skb, info);
@@ -2600,7 +2600,6 @@ static int devlink_resource_fill(struct genl_info *info,
nla_put_failure:
err = -EMSGSIZE;
err_resource_put:
-err_skb_send_alloc:
nlmsg_free(skb);
return err;
}
[PATCH] ixgbevf: off by one in ixgbevf_ipsec_tx()
The ipsec->tx_tbl[] array has IXGBE_IPSEC_MAX_SA_COUNT elements so the >
should be a >=.
Fixes: 0062e7cc955e ("ixgbevf: add VF IPsec offload code")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
index 997cea675a37..4fcbeffce52b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
@@ -470,7 +470,7 @@ int ixgbevf_ipsec_tx(struct ixgbevf_ring *tx_ring,
}
sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
- if (unlikely(sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
+ if (unlikely(sa_idx >= IXGBE_IPSEC_MAX_SA_COUNT)) {
netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
__func__, sa_idx, xs->xso.offload_handle);
return 0;
[PATCH net-next] net: dsa: b53: Uninitialized variable in b53_adjust_link()
The "pause" variable is only initialized on BCM5301x.
Fixes: 5e004460f874 ("net: dsa: b53: Add helper to set link parameters")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index ea4256cd628b..dbf5b86a07fe 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1025,7 +1025,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int
port,
struct b53_device *dev = ds->priv;
struct ethtool_eee *p = >ports[port].eee;
u8 rgmii_ctrl = 0, reg = 0, off;
- int pause;
+ int pause = 0;
if (!phy_is_pseudo_fixed_link(phydev))
return;
Re: [PATCH] ieee802154: mcr20a: read out of bounds in mcr20a_set_channel()
On Wed, Aug 29, 2018 at 07:50:51PM +0200, Xue Liu wrote: > Hello Dan, > > > On Wed, 29 Aug 2018 at 16:49, Dan Carpenter > wrote: > > > The "channel" variable can be any u8 value. We need to make sure we > > don't read outside of the PLL_INT[] or PLL_FRAC[] arrays. > > > I think the “channel” variable can not be any u8 value. This values is > already checked before set_channel function is called. > https://github.com/torvalds/linux/blob/master/net/ieee802154/nl802154.c#L978 Oh... Hm... I should have reviewed more carefully. This patch isn't correct. But there may still be a bug. What Smatch is worried about is the other call tree: ieee802154_start_req() calls (struct ieee802154_mlme_ops)->start_req -> mac802154_mlme_start_req() -> mac802154_dev_set_page_channel() -> drv_set_channel() calls local->ops->set_channel(>hw, page, channel); -> mcr20a_set_channel() So maybe we could move the check from nl802154_set_channel() to drv_set_channel() so channel is checked on both call trees. regards, dan carpenter
[PATCH] ieee802154: mcr20a: read out of bounds in mcr20a_set_channel()
The "channel" variable can be any u8 value. We need to make sure we
don't read outside of the PLL_INT[] or PLL_FRAC[] arrays.
Fixes: 8c6ad9cc5157 ("ieee802154: Add NXP MCR20A IEEE 802.15.4 transceiver
driver")
Signed-off-by: Dan Carpenter
---
This patch is obviously harmless, but it's from static analysis. I'm
pretty sure this is required, but I can't swear.
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index e428277781ac..4f41d1d3588e 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -512,6 +512,9 @@ mcr20a_set_channel(struct ieee802154_hw *hw, u8 page, u8
channel)
dev_dbg(printdev(lp), "%s\n", __func__);
+ if (channel < 11 || channel - 11 >= ARRAY_SIZE(PLL_INT))
+ return -EINVAL;
+
/* freqency = ((PLL_INT+64) + (PLL_FRAC/65536)) * 32 MHz */
ret = regmap_write(lp->regmap_dar, DAR_PLL_INT0, PLL_INT[channel - 11]);
if (ret)
[PATCH net-next] net: dsa: mv88e6xxx: missing unlock on error path
We added a new error path, but we need to drop the lock before we return.
Fixes: 2d2e1dd29962 ("net: dsa: mv88e6xxx: Cache the port cmode")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 17752316ab10..8da3d39e3218 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2408,7 +2408,7 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
if (chip->info->ops->port_get_cmode) {
err = chip->info->ops->port_get_cmode(chip, i, );
if (err)
- return err;
+ goto unlock;
chip->ports[i].cmode = cmode;
}
[PATCH net-next] net: dsa: mv88e6xxx: bitwise vs logical bug
We are trying to test if these flags are set but there are some && vs &
typos.
Fixes: efd1ba6af93f ("net: dsa: mv88e6xxx: Add SERDES phydev_mac_change up for
6390")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c
b/drivers/net/dsa/mv88e6xxx/serdes.c
index f007d109b385..e82983975754 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -502,8 +502,8 @@ static irqreturn_t mv88e6390_serdes_thread_fn(int irq, void
*dev_id)
err = mv88e6390_serdes_irq_status_sgmii(chip, lane, );
if (err)
goto out;
- if (status && (MV88E6390_SGMII_INT_LINK_DOWN ||
- MV88E6390_SGMII_INT_LINK_UP)) {
+ if (status & (MV88E6390_SGMII_INT_LINK_DOWN |
+ MV88E6390_SGMII_INT_LINK_UP)) {
ret = IRQ_HANDLED;
mv88e6390_serdes_irq_link_sgmii(chip, port->port, lane);
}
[PATCH net-next] ipv4: frags: precedence bug in ip_expire()
We accidentally removed the parentheses here, but they are required
because '!' has higher precedence than '&'.
Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Signed-off-by: Dan Carpenter
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 0e8f8de77e71..7cb7ed761d8c 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -154,7 +154,7 @@ static void ip_expire(struct timer_list *t)
__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
__IP_INC_STATS(net, IPSTATS_MIB_REASMTIMEOUT);
- if (!qp->q.flags & INET_FRAG_FIRST_IN)
+ if (!(qp->q.flags & INET_FRAG_FIRST_IN))
goto out;
/* sk_buff::dev and sk_buff::rbnode are unionized. So we
[PATCH net-next] net: sched: cls_flower: Fix an error code in fl_tmplt_create()
We forgot to set the error code on this path, so we return NULL instead
of an error pointer. In the current code kzalloc() won't fail for small
allocations so this doesn't really affect runtime.
Fixes: b95ec7eb3b4d ("net: sched: cls_flower: implement chain templates")
Signed-off-by: Dan Carpenter
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index e8bd08ba998a..a3b69bb6f4b0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1250,8 +1250,10 @@ static void *fl_tmplt_create(struct net *net, struct
tcf_chain *chain,
goto errout_tb;
tmplt = kzalloc(sizeof(*tmplt), GFP_KERNEL);
- if (!tmplt)
+ if (!tmplt) {
+ err = -ENOMEM;
goto errout_tb;
+ }
tmplt->chain = chain;
err = fl_set_key(net, tb, >dummy_key, >mask, extack);
if (err)
Re: [PATCH 1/2] samples: bpf: ensure that we don't load over MAX_PROGS programs
On Fri, Jul 13, 2018 at 04:13:30PM +0100, Colin Ian King wrote: > On 13/07/18 16:11, Dan Carpenter wrote: > > I can't see that we check prog_cnt to ensure it doesn't go over > > MAX_PROGS. > > > > Signed-off-by: Dan Carpenter > > > > diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c > > index 89161c9ed466..904e775d1a44 100644 > > --- a/samples/bpf/bpf_load.c > > +++ b/samples/bpf/bpf_load.c > > @@ -107,6 +107,9 @@ static int load_and_attach(const char *event, struct > > bpf_insn *prog, int size) > > return -1; > > } > > > > + if (prog_cnt == MAX_PROGS) > > + return -1; > > + > > Should that be "if (prog_cnt >= MAX_PROGS)" ? It's incremented one at a time so it can't go over. regards, dan carpenter
[PATCH 1/2] samples: bpf: ensure that we don't load over MAX_PROGS programs
I can't see that we check prog_cnt to ensure it doesn't go over
MAX_PROGS.
Signed-off-by: Dan Carpenter
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 89161c9ed466..904e775d1a44 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -107,6 +107,9 @@ static int load_and_attach(const char *event, struct
bpf_insn *prog, int size)
return -1;
}
+ if (prog_cnt == MAX_PROGS)
+ return -1;
+
fd = bpf_load_program(prog_type, prog, insns_cnt, license, kern_version,
bpf_log_buf, BPF_LOG_BUF_SIZE);
if (fd < 0) {
[PATCH 2/2] samples/bpf: test_cgrp2_sock2: fix an off by one
"prog_cnt" is the number of elements which are filled out in prog_fd[]
so the test should be >= instead of >.
Signed-off-by: Dan Carpenter
diff --git a/samples/bpf/test_cgrp2_sock2.c b/samples/bpf/test_cgrp2_sock2.c
index 3b5be2364975..a9277b118c33 100644
--- a/samples/bpf/test_cgrp2_sock2.c
+++ b/samples/bpf/test_cgrp2_sock2.c
@@ -51,7 +51,7 @@ int main(int argc, char **argv)
if (argc > 3)
filter_id = atoi(argv[3]);
- if (filter_id > prog_cnt) {
+ if (filter_id >= prog_cnt) {
printf("Invalid program id; program not found in file\n");
return EXIT_FAILURE;
}
[PATCH net] qlogic: check kstrtoul() for errors
We accidentally left out the error handling for kstrtoul().
Fixes: a520030e326a ("qlcnic: Implement flash sysfs callback for 83xx adapter")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index 891f03a7a33d..8d7b9bb910f2 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -1128,6 +1128,8 @@ static ssize_t
qlcnic_83xx_sysfs_flash_write_handler(struct file *filp,
struct qlcnic_adapter *adapter = dev_get_drvdata(dev);
ret = kstrtoul(buf, 16, );
+ if (ret)
+ return ret;
switch (data) {
case QLC_83XX_FLASH_SECTOR_ERASE_CMD:
[PATCH net] ixgbe: Off by one in ixgbe_ipsec_tx()
The ipsec->tx_tbl[] has IXGBE_IPSEC_MAX_SA_COUNT elements so the > needs
to be changed to >= so we don't read one element beyond the end of the
array.
Fixes: 592594704761 ("ixgbe: process the Tx ipsec offload")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index c116f459945d..da4322e4daed 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -839,7 +839,7 @@ int ixgbe_ipsec_tx(struct ixgbe_ring *tx_ring,
}
itd->sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
- if (unlikely(itd->sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
+ if (unlikely(itd->sa_idx >= IXGBE_IPSEC_MAX_SA_COUNT)) {
netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
__func__, itd->sa_idx, xs->xso.offload_handle);
return 0;
[PATCH net] qed: off by one in qed_parse_mcp_trace_buf()
If format_idx == s_mcp_trace_meta.formats_num then we read one element
beyond the end of the s_mcp_trace_meta.formats[] array.
Fixes: 50bc60cb155c ("qed*: Utilize FW 8.33.11.0")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c
b/drivers/net/ethernet/qlogic/qed/qed_debug.c
index a14e48489029..4340c4c90bcb 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
@@ -6723,7 +6723,7 @@ static enum dbg_status qed_parse_mcp_trace_buf(u8
*trace_buf,
format_idx = header & MFW_TRACE_EVENTID_MASK;
/* Skip message if its index doesn't exist in the meta data */
- if (format_idx > s_mcp_trace_meta.formats_num) {
+ if (format_idx >= s_mcp_trace_meta.formats_num) {
u8 format_size =
(u8)((header & MFW_TRACE_PRM_SIZE_MASK) >>
MFW_TRACE_PRM_SIZE_SHIFT);
[PATCH net] cnic: tidy up a size calculation
Static checkers complain that id_tbl->table points to longs and 4 bytes is smaller than sizeof(long). But the since other side is dividing by 32 instead of sizeof(long), that means the current code works fine. Anyway, it's more conventional to use the BITS_TO_LONGS() macro when we're allocating a bitmap. Signed-off-by: Dan Carpenter diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c index 30273a7717e2..4fd829b5e65d 100644 --- a/drivers/net/ethernet/broadcom/cnic.c +++ b/drivers/net/ethernet/broadcom/cnic.c @@ -660,7 +660,7 @@ static int cnic_init_id_tbl(struct cnic_id_tbl *id_tbl, u32 size, u32 start_id, id_tbl->max = size; id_tbl->next = next; spin_lock_init(_tbl->lock); - id_tbl->table = kcalloc(DIV_ROUND_UP(size, 32), 4, GFP_KERNEL); + id_tbl->table = kcalloc(BITS_TO_LONGS(size), sizeof(long), GFP_KERNEL); if (!id_tbl->table) return -ENOMEM;
[PATCH net] atm: iphase: fix a 64 bit bug
The code assumes that there is 4 bytes in a pointer and it doesn't
allocate enough memory.
Signed-off-by: Dan Carpenter
diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index ff81a576347e..82532c299bb5 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -1618,7 +1618,7 @@ static int rx_init(struct atm_dev *dev)
skb_queue_head_init(>rx_dma_q);
iadev->rx_free_desc_qhead = NULL;
- iadev->rx_open = kcalloc(4, iadev->num_vc, GFP_KERNEL);
+ iadev->rx_open = kcalloc(iadev->num_vc, sizeof(void *), GFP_KERNEL);
if (!iadev->rx_open) {
printk(KERN_ERR DEV_LABEL "itf %d couldn't get free page\n",
dev->number);
[bug report] i40e: main driver core
Hello Jesse Brandeburg,
The patch 41c445ff0f48: "i40e: main driver core" from Sep 11, 2013,
leads to the following static checker warning:
drivers/net/ethernet/intel/i40e/i40e_main.c:13065 i40e_veb_setup()
warn: potential off by one 'pf->vsi[]' 'vsi_idx == pf->num_alloc_vsi'
drivers/net/ethernet/intel/i40e/i40e_main.c
13025 uplink_seid, vsi_seid);
13026 return NULL;
13027 }
13028
13029 /* make sure there is such a vsi and uplink */
13030 for (vsi_idx = 0; vsi_idx < pf->num_alloc_vsi; vsi_idx++)
13031 if (pf->vsi[vsi_idx] && pf->vsi[vsi_idx]->seid ==
vsi_seid)
13032 break;
13033 if (vsi_idx >= pf->num_alloc_vsi && vsi_seid != 0) {
^
I think if pf->num_alloc_vsi is zero then we're going to run into
problems. We should remove this condition.
13034 dev_info(>pdev->dev, "vsi seid %d not found\n",
13035 vsi_seid);
13036 return NULL;
13037 }
13038
13039 if (uplink_seid && uplink_seid != pf->mac_seid) {
13040 for (veb_idx = 0; veb_idx < I40E_MAX_VEB; veb_idx++) {
13041 if (pf->veb[veb_idx] &&
13042 pf->veb[veb_idx]->seid == uplink_seid) {
13043 uplink_veb = pf->veb[veb_idx];
13044 break;
13045 }
13046 }
13047 if (!uplink_veb) {
13048 dev_info(>pdev->dev,
13049 "uplink seid %d not found\n",
uplink_seid);
13050 return NULL;
13051 }
13052 }
13053
13054 /* get veb sw struct */
13055 veb_idx = i40e_veb_mem_alloc(pf);
13056 if (veb_idx < 0)
13057 goto err_alloc;
13058 veb = pf->veb[veb_idx];
13059 veb->flags = flags;
13060 veb->uplink_seid = uplink_seid;
13061 veb->veb_idx = (uplink_veb ? uplink_veb->idx : I40E_NO_VEB);
13062 veb->enabled_tc = (enabled_tc ? enabled_tc : 0x1);
13063
13064 /* create the VEB in the switch */
13065 ret = i40e_add_veb(veb, pf->vsi[vsi_idx]);
13066 if (ret)
13067 goto err_veb;
regards,
dan carpenter
Re: [PATCH net 2/3] hv_netvsc: fix network namespace issues with VF support
On Mon, Jun 11, 2018 at 12:44:55PM -0700, Stephen Hemminger wrote:
> When finding the parent netvsc device, the search needs to be across
> all netvsc device instances (independent of network namespace).
>
> Find parent device of VF using upper_dev_get routine which
> searches only adjacent list.
>
> Fixes: e8ff40d4bff1 ("hv_netvsc: improve VF device matching")
> Signed-off-by: Stephen Hemminger
>
> netns aware byref
What? Presumably that wasn't supposed to be part of the commit message.
> ---
regards,
dan carpenter
[PATCH 2/2 net] team: use netdev_features_t instead of u32
This code was introduced in 2011 around the same time that we made
netdev_features_t a u64 type. These days a u32 is not big enough to
hold all the potential features.
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 267dcc929f6c..8863fa023500 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1004,7 +1004,8 @@ static void team_port_disable(struct team *team,
static void __team_compute_features(struct team *team)
{
struct team_port *port;
- u32 vlan_features = TEAM_VLAN_FEATURES & NETIF_F_ALL_FOR_ALL;
+ netdev_features_t vlan_features = TEAM_VLAN_FEATURES &
+ NETIF_F_ALL_FOR_ALL;
netdev_features_t enc_features = TEAM_ENC_FEATURES;
unsigned short max_hard_header_len = ETH_HLEN;
unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
[PATCH 1/2 v2 net-next] net_failover: Use netdev_features_t instead of u32
The features mask needs to be a netdev_features_t (u64) because a u32
is not big enough.
Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
Signed-off-by: Dan Carpenter
---
v2: In the original patch, I thought that the & should be | and I
introduced a bug.
diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 8b508e2cf29b..83f7420ddea5 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -380,7 +380,8 @@ static rx_handler_result_t net_failover_handle_frame(struct
sk_buff **pskb)
static void net_failover_compute_features(struct net_device *dev)
{
- u32 vlan_features = FAILOVER_VLAN_FEATURES & NETIF_F_ALL_FOR_ALL;
+ netdev_features_t vlan_features = FAILOVER_VLAN_FEATURES &
+ NETIF_F_ALL_FOR_ALL;
netdev_features_t enc_features = FAILOVER_ENC_FEATURES;
unsigned short max_hard_header_len = ETH_HLEN;
unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
[PATCH 2/2 net-next] net_failover: fix error code in net_failover_create()
We forgot to set the error code on this path. This function is supposed
to return error pointers, so with this bug it accidentally returns NULL
and the caller doesn't check for that.
Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index ef50158e90a9..881f3fa13e6b 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -761,8 +761,10 @@ struct failover *net_failover_create(struct net_device
*standby_dev)
netif_carrier_off(failover_dev);
failover = failover_register(failover_dev, _failover_ops);
- if (IS_ERR(failover))
+ if (IS_ERR(failover)) {
+ err = PTR_ERR(failover);
goto err_failover_register;
+ }
return failover;
[PATCH 1/2 net-next] net_failover: fix net_failover_compute_features()
This has an '&' vs '|' typo so it starts with vlan_features set to none.
Also a u32 type isn't large enough to hold all the feature bits, it
should be netdev_features_t.
Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c
index 8b508e2cf29b..ef50158e90a9 100644
--- a/drivers/net/net_failover.c
+++ b/drivers/net/net_failover.c
@@ -380,7 +380,8 @@ static rx_handler_result_t net_failover_handle_frame(struct
sk_buff **pskb)
static void net_failover_compute_features(struct net_device *dev)
{
- u32 vlan_features = FAILOVER_VLAN_FEATURES & NETIF_F_ALL_FOR_ALL;
+ netdev_features_t vlan_features = FAILOVER_VLAN_FEATURES |
+ NETIF_F_ALL_FOR_ALL;
netdev_features_t enc_features = FAILOVER_ENC_FEATURES;
unsigned short max_hard_header_len = ETH_HLEN;
unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |
[PATCH -net] net: ethernet: davinci_emac: fix error handling in probe()
The current error handling code has an issue where it does:
if (priv->txchan)
cpdma_chan_destroy(priv->txchan);
The problem is that ->txchan is either valid or an error pointer (which
would lead to an Oops). I've changed it to use multiple error labels so
that the test can be removed.
Also there were some missing calls to netif_napi_del().
Fixes: 3ef0fdb2342c ("net: davinci_emac: switch to new cpdma layer")
Signed-off-by: Dan Carpenter
diff --git a/drivers/net/ethernet/ti/davinci_emac.c
b/drivers/net/ethernet/ti/davinci_emac.c
index be0fec17d95d..06d7c9e4dcda 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -1873,7 +1873,7 @@ static int davinci_emac_probe(struct platform_device
*pdev)
if (IS_ERR(priv->txchan)) {
dev_err(>dev, "error initializing tx dma channel\n");
rc = PTR_ERR(priv->txchan);
- goto no_cpdma_chan;
+ goto err_free_dma;
}
priv->rxchan = cpdma_chan_create(priv->dma, EMAC_DEF_RX_CH,
@@ -1881,14 +1881,14 @@ static int davinci_emac_probe(struct platform_device
*pdev)
if (IS_ERR(priv->rxchan)) {
dev_err(>dev, "error initializing rx dma channel\n");
rc = PTR_ERR(priv->rxchan);
- goto no_cpdma_chan;
+ goto err_free_txchan;
}
res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
if (!res) {
dev_err(>dev, "error getting irq res\n");
rc = -ENOENT;
- goto no_cpdma_chan;
+ goto err_free_rxchan;
}
ndev->irq = res->start;
@@ -1914,7 +1914,7 @@ static int davinci_emac_probe(struct platform_device
*pdev)
pm_runtime_put_noidle(>dev);
dev_err(>dev, "%s: failed to get_sync(%d)\n",
__func__, rc);
- goto no_cpdma_chan;
+ goto err_napi_del;
}
/* register the network device */
@@ -1924,7 +1924,7 @@ static int davinci_emac_probe(struct platform_device
*pdev)
dev_err(>dev, "error in register_netdev\n");
rc = -ENODEV;
pm_runtime_put(>dev);
- goto no_cpdma_chan;
+ goto err_napi_del;
}
@@ -1937,11 +1937,13 @@ static int davinci_emac_probe(struct platform_device
*pdev)
return 0;
-no_cpdma_chan:
- if (priv->txchan)
- cpdma_chan_destroy(priv->txchan);
- if (priv->rxchan)
- cpdma_chan_destroy(priv->rxchan);
+err_napi_del:
+ netif_napi_del(>napi);
+err_free_rxchan:
+ cpdma_chan_destroy(priv->rxchan);
+err_free_txchan:
+ cpdma_chan_destroy(priv->txchan);
+err_free_dma:
cpdma_ctlr_destroy(priv->dma);
no_pdata:
if (of_phy_is_fixed_link(np))
Re: [PATCH 1/2] bpf: sockmap, double free in __sock_map_ctx_update_elem()
On Fri, May 18, 2018 at 10:27:18AM +0200, Daniel Borkmann wrote: > > Thanks for the two fixes, appreciate it! There were two similar ones that > fix the same issues which were already applied yesterday to bpf-next: > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=0e4364560361d57e8cd873a8990327f3471d7d8a > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=a78622932c27e8ec33e5ba180f3d2e87fb806b28 Hey Gustavo, We're sort of duplicating each other's work. Could you CC kernel-janit...@vger.kernel.org for static checker fixes so that I can see what you're working on? We'll probably still send the occasional duplicate which is fine... regards, dan carpenter
[PATCH 2/2] bpf: sockmap, potential uninitialized return in __sock_map_ctx_update_elem()
Smatch complains that "err" might be uninitialized. That seems
possible. Anyway we can just return zero.
Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work with
hashmap")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index c6de1393df63..6298adb3162a 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1821,7 +1821,7 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
list_add_tail(>list, >maps);
}
write_unlock_bh(>sk_callback_lock);
- return err;
+ return 0;
out_free:
kfree(e);
smap_release_sock(psock, sock);
[PATCH 1/2] bpf: sockmap, double free in __sock_map_ctx_update_elem()
We accidentally free "e" twice.
Fixes: 81110384441a ("bpf: sockmap, add hash map support")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index c6de1393df63..216d5c9b0eb3 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1833,7 +1833,6 @@ static int __sock_map_ctx_update_elem(struct bpf_map *map,
if (tx_msg)
bpf_prog_put(tx_msg);
write_unlock_bh(>sk_callback_lock);
- kfree(e);
return err;
}
[PATCH] net/ncsi: prevent a couple array underflows
We recently refactored this code and introduced a static checker
warning. Smatch complains that if cmd->index is zero then we would
underflow the arrays. That's obviously true.
The question is whether we prevent cmd->index from being zero at a
different level. I've looked at the code and I don't immediately see
a check for that.
Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index ce9497966ebe..a6b7c7d5c829 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -347,7 +347,7 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr)
cmd = (struct ncsi_cmd_svf_pkt *)skb_network_header(nr->cmd);
ncf = >vlan_filter;
- if (cmd->index > ncf->n_vids)
+ if (cmd->index == 0 || cmd->index > ncf->n_vids)
return -ERANGE;
/* Add or remove the VLAN filter. Remember HW indexes from 1 */
@@ -445,7 +445,8 @@ static int ncsi_rsp_handler_sma(struct ncsi_request *nr)
ncf = >mac_filter;
bitmap = >bitmap;
- if (cmd->index > ncf->n_uc + ncf->n_mc + ncf->n_mixed)
+ if (cmd->index == 0 ||
+ cmd->index > ncf->n_uc + ncf->n_mc + ncf->n_mixed)
return -ERANGE;
index = (cmd->index - 1) * ETH_ALEN;
Re: [PATCH 2/5] crypto: chtls: wait for memory sendmsg, sendpage
On Mon, May 14, 2018 at 04:30:56PM +0530, Atul Gupta wrote:
> Reported-by: Gustavo A. R. Silva <gust...@embeddedor.com>
> Signed-off-by: Atul Gupta <atul.gu...@chelsio.com>
There isn't a commit message for this. It should say what the user
visible effects of this bug are. I haven't seen Gustavo's bug report,
but probably copy and pasting that would be good?
> ---
> drivers/crypto/chelsio/chtls/chtls.h | 1 +
> drivers/crypto/chelsio/chtls/chtls_io.c | 90
> +--
> drivers/crypto/chelsio/chtls/chtls_main.c | 1 +
> 3 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chtls/chtls.h
> b/drivers/crypto/chelsio/chtls/chtls.h
> index f4b8f1e..778c194 100644
> --- a/drivers/crypto/chelsio/chtls/chtls.h
> +++ b/drivers/crypto/chelsio/chtls/chtls.h
> @@ -149,6 +149,7 @@ struct chtls_dev {
> struct list_head rcu_node;
> struct list_head na_node;
> unsigned int send_page_order;
> + int max_host_sndbuf;
> struct key_map kmap;
> };
>
> diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c
> b/drivers/crypto/chelsio/chtls/chtls_io.c
> index 5a75be4..a4c7d2d 100644
> --- a/drivers/crypto/chelsio/chtls/chtls_io.c
> +++ b/drivers/crypto/chelsio/chtls/chtls_io.c
> @@ -914,6 +914,78 @@ static u16 tls_header_read(struct tls_hdr *thdr, struct
> iov_iter *from)
> return (__force u16)cpu_to_be16(thdr->length);
> }
>
> +static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
> +{
> + return (cdev->max_host_sndbuf - sk->sk_wmem_queued) > 0;
Why not just say:
return (max_host_sndbuf > sk->sk_wmem_queued);
> +}
> +
> +static int csk_wait_memory(struct chtls_dev *cdev,
> +struct sock *sk, long *timeo_p)
> +{
> + DEFINE_WAIT_FUNC(wait, woken_wake_function);
> + int sndbuf, err = 0;
> + long current_timeo;
> + long vm_wait = 0;
> + bool noblock;
> +
> + current_timeo = *timeo_p;
> + noblock = (*timeo_p ? false : true);
> sndbuf = cdev->max_host_sndbuf;
> + if (sndbuf > sk->sk_wmem_queued) {
You could use it here:
if (csk_mem_free(cdev, sk)) {
> + current_timeo = (prandom_u32() % (HZ / 5)) + 2;
> + vm_wait = (prandom_u32() % (HZ / 5)) + 2;
> + }
> +
> + add_wait_queue(sk_sleep(sk), );
> + while (1) {
> + sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
> +
> + if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
> + goto do_error;
> + if (!*timeo_p) {
> + if (noblock)
> + set_bit(SOCK_NOSPACE, >sk_socket->flags);
> + goto do_nonblock;
There are missing curly braces here. I feel like these gotos make the
code worse. They don't remove duplicate lines of code. They just
spread things out so that you have to jump around to understand this
code. It's like being a kangaroo.
if (noblock) {
set_bit(SOCK_NOSPACE, >sk_socket->flags);
err = -EAGAIN;
goto remove_queue;
}
I always like picking a descriptive label names instead of "out:"
> + }
> + if (signal_pending(current))
> + goto do_interrupted;
> + sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
> + if (sndbuf > sk->sk_wmem_queued && !vm_wait)
> + break;
if (csk_mem_free(cdev, sk) && !vm_wait)
> +
> + set_bit(SOCK_NOSPACE, >sk_socket->flags);
> + sk->sk_write_pending++;
> + sk_wait_event(sk, _timeo, sk->sk_err ||
> + (sk->sk_shutdown & SEND_SHUTDOWN) ||
> + (sndbuf > sk->sk_wmem_queued && !vm_wait), );
(csk_mem_free(cdev, sk) && !vm_wait), );
> + sk->sk_write_pending--;
> +
> + if (vm_wait) {
> + vm_wait -= current_timeo;
> + current_timeo = *timeo_p;
> + if (current_timeo != MAX_SCHEDULE_TIMEOUT) {
> + current_timeo -= vm_wait;
> + if (current_timeo < 0)
> + current_timeo = 0;
> + }
> + vm_wait = 0;
> + }
> + *timeo_p = current_timeo;
> + }
> +out:
> + remove_wait_queue(sk_sleep(sk), );
> + return err;
> +do_error:
> + err = -EPIPE;
> + goto out;
> +do_nonblock:
> + err = -EAGAIN;
> + goto out;
> +do_interrupted:
> + err = sock_intr_errno(*timeo_p);
> + goto out;
> +}
> +
regards,
dan carpenter
Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote: > On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote: > > If an error occurs, 'mlx4_en_destroy_netdev()' is called. > > It then calls 'mlx4_en_free_resources()' which does the needed resources > > cleanup. > > > > So, doing some explicit kfree in the error handling path would lead to > > some double kfree. > > Patch make sense but what's bothering me is that mlx4_en_free_resources > loops on the entire array, assuming !priv->tx_ring[t] means entry is > allocated but the existing code does not assume that, see [1]. So i looked > to see where tx_ring array is zeroed and didn't find it. > > Am i missing something here. > It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and then we do a memset(priv, 0, sizeof(struct mlx4_en_priv)); regards, dan carpenter
Re: [PATCH] mlxsw: core: Fix an error handling path in \'mlxsw_core_bus_device_register()\'
On Thu, May 10, 2018 at 04:08:22PM +0300, Arkadi Sharshevsky wrote: > Hi Dan, > > I will fix the error path. Regarding the goto label this is > the convention in the driver. There is no rule against learning from the past. Or there might be... I don't know all the rules at Mellanox. regards, dan carpenter
Re: [PATCH] mlxsw: core: Fix an error handling path in 'mlxsw_core_bus_device_register()'
On Thu, May 10, 2018 at 01:26:16PM +0200, Christophe JAILLET wrote: > Resources are not freed in the reverse order of the allocation. > Labels are also mixed-up. > > Fix it and reorder code and labels in the error handling path of > 'mlxsw_core_bus_device_register()' > > Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr> > --- > Please review carefully. This patch is proposed because it triggers one of > my coccinelle scripts. I'm not 100% sure if correct. > Looks correct. The err = mlxsw_driver->resources_register(mlxsw_core); pointer is a pointer to mlxsw_sp_resources_register(). That function doesn't clean up after itself on failure. Ideally, you'd want a matching mlxsw_driver->resources_unregister as well pointer instead of hard coding devlink_resources_unregister(). The error handling would be easier to review if the gotos told you what the did. Right now they're written in "come from" style so the tell you what happened on the line before. if (!foo) goto allocating_foo_failed; Hopefully if someone fixes mlxsw_sp_resources_register() they'll choose better label names. regards, dan carpenter
Re: [PATCH] netfilter: nf_queue: Replace conntrack entry
Hi Kristian,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on nf-next/master]
[also build test WARNING on v4.17-rc3 next-20180504]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Kristian-Evensen/netfilter-nf_queue-Replace-conntrack-entry/20180504-051218
base: https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
smatch warnings:
net/netfilter/nfnetlink_queue.c:1141 nfqnl_recv_verdict_batch() warn: curly
braces intended?
#
https://github.com/0day-ci/linux/commit/8776e32a6c6e2ba0c6c8ce85e227672b81a1649d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8776e32a6c6e2ba0c6c8ce85e227672b81a1649d
vim +1141 net/netfilter/nfnetlink_queue.c
8776e32a net/netfilter/nfnetlink_queue.c Kristian Evensen 2018-05-03
1093
7b8002a1 net/netfilter/nfnetlink_queue.c Pablo Neira Ayuso 2015-12-15
1094 static int nfqnl_recv_verdict_batch(struct net *net, struct sock *ctnl,
7b8002a1 net/netfilter/nfnetlink_queue.c Pablo Neira Ayuso 2015-12-15
1095struct sk_buff *skb,
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1096const struct nlmsghdr *nlh,
04ba724b net/netfilter/nfnetlink_queue.c Pablo Neira Ayuso 2017-06-19
1097const struct nlattr * const nfqa[],
04ba724b net/netfilter/nfnetlink_queue.c Pablo Neira Ayuso 2017-06-19
1098struct netlink_ext_ack *extack)
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1099 {
3da07c0c net/netfilter/nfnetlink_queue_core.c David S. Miller 2012-06-26
1100struct nfgenmsg *nfmsg = nlmsg_data(nlh);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1101struct nf_queue_entry *entry, *tmp;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1102unsigned int verdict, maxid;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1103struct nfqnl_msg_verdict_hdr *vhdr;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1104struct nfqnl_instance *queue;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1105LIST_HEAD(batch_list);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1106u16 queue_num = ntohs(nfmsg->res_id);
e8179610 net/netfilter/nfnetlink_queue_core.c Gao feng 2013-03-24
1107struct nfnl_queue_net *q = nfnl_queue_pernet(net);
8776e32a net/netfilter/nfnetlink_queue.c Kristian Evensen 2018-05-03
1108enum ip_conntrack_info ctinfo;
e8179610 net/netfilter/nfnetlink_queue_core.c Gao feng 2013-03-24
1109
e8179610 net/netfilter/nfnetlink_queue_core.c Gao feng 2013-03-24
1110queue = verdict_instance_lookup(q, queue_num,
e8179610 net/netfilter/nfnetlink_queue_core.c Gao feng 2013-03-24
NETLINK_CB(skb).portid);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1112if (IS_ERR(queue))
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1113return PTR_ERR(queue);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1114
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1115vhdr = verdicthdr_get(nfqa);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1116if (!vhdr)
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1117return -EINVAL;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1118
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1119verdict = ntohl(vhdr->verdict);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1120maxid = ntohl(vhdr->id);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1121
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1122spin_lock_bh(>lock);
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1123
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1124list_for_each_entry_safe(entry, tmp, >queue_list, list) {
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1125if (nfq_id_after(entry->id, maxid))
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1126break;
97d32cf9 net/netfilter/nfnetlink_queue.c Florian Westphal 2011-07-19
1127
Re: [PATCH net-next v8 2/4] net: Introduce generic failover module
Hi Sridhar,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url:
https://github.com/0day-ci/linux/commits/Sridhar-Samudrala/Enable-virtio_net-to-act-as-a-standby-for-a-passthru-device/20180427-183842
smatch warnings:
net/core/net_failover.c:229 net_failover_change_mtu() error: we previously
assumed 'primary_dev' could be null (see line 219)
net/core/net_failover.c:279 net_failover_vlan_rx_add_vid() error: we previously
assumed 'primary_dev' could be null (see line 269)
#
https://github.com/0day-ci/linux/commit/5a5f2e3efcb699867db79543dfebe764927b9c93
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 5a5f2e3efcb699867db79543dfebe764927b9c93
vim +/primary_dev +229 net/core/net_failover.c
5a5f2e3e Sridhar Samudrala 2018-04-25 211
5a5f2e3e Sridhar Samudrala 2018-04-25 212 static int
net_failover_change_mtu(struct net_device *dev, int new_mtu)
5a5f2e3e Sridhar Samudrala 2018-04-25 213 {
5a5f2e3e Sridhar Samudrala 2018-04-25 214 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 215 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 216 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 217
5a5f2e3e Sridhar Samudrala 2018-04-25 218 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 @219 if (primary_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 220 ret =
dev_set_mtu(primary_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 221 if (ret)
5a5f2e3e Sridhar Samudrala 2018-04-25 222 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 223 }
5a5f2e3e Sridhar Samudrala 2018-04-25 224
5a5f2e3e Sridhar Samudrala 2018-04-25 225 standby_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 226 if (standby_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 227 ret =
dev_set_mtu(standby_dev, new_mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 228 if (ret) {
5a5f2e3e Sridhar Samudrala 2018-04-25 @229
dev_set_mtu(primary_dev, dev->mtu);
5a5f2e3e Sridhar Samudrala 2018-04-25 230 return ret;
5a5f2e3e Sridhar Samudrala 2018-04-25 231 }
5a5f2e3e Sridhar Samudrala 2018-04-25 232 }
5a5f2e3e Sridhar Samudrala 2018-04-25 233
5a5f2e3e Sridhar Samudrala 2018-04-25 234 dev->mtu = new_mtu;
5a5f2e3e Sridhar Samudrala 2018-04-25 235
5a5f2e3e Sridhar Samudrala 2018-04-25 236 return 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 237 }
5a5f2e3e Sridhar Samudrala 2018-04-25 238
5a5f2e3e Sridhar Samudrala 2018-04-25 239 static void
net_failover_set_rx_mode(struct net_device *dev)
5a5f2e3e Sridhar Samudrala 2018-04-25 240 {
5a5f2e3e Sridhar Samudrala 2018-04-25 241 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 242 struct net_device *slave_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 243
5a5f2e3e Sridhar Samudrala 2018-04-25 244 rcu_read_lock();
5a5f2e3e Sridhar Samudrala 2018-04-25 245
5a5f2e3e Sridhar Samudrala 2018-04-25 246 slave_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 247 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 248
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 249
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 250 }
5a5f2e3e Sridhar Samudrala 2018-04-25 251
5a5f2e3e Sridhar Samudrala 2018-04-25 252 slave_dev =
rcu_dereference(nfo_info->standby_dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 253 if (slave_dev) {
5a5f2e3e Sridhar Samudrala 2018-04-25 254
dev_uc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 255
dev_mc_sync_multiple(slave_dev, dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 256 }
5a5f2e3e Sridhar Samudrala 2018-04-25 257
5a5f2e3e Sridhar Samudrala 2018-04-25 258 rcu_read_unlock();
5a5f2e3e Sridhar Samudrala 2018-04-25 259 }
5a5f2e3e Sridhar Samudrala 2018-04-25 260
5a5f2e3e Sridhar Samudrala 2018-04-25 261 static int
net_failover_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
5a5f2e3e Sridhar Samudrala 2018-04-25 262
u16 vid)
5a5f2e3e Sridhar Samudrala 2018-04-25 263 {
5a5f2e3e Sridhar Samudrala 2018-04-25 264 struct net_failover_info
*nfo_info = netdev_priv(dev);
5a5f2e3e Sridhar Samudrala 2018-04-25 265 struct net_device *primary_dev,
*standby_dev;
5a5f2e3e Sridhar Samudrala 2018-04-25 266 int ret = 0;
5a5f2e3e Sridhar Samudrala 2018-04-25 267
5a5f2e3e Sridhar Samudrala 2018-04-25 268 primary_dev =
rcu_dereference(nfo_info->primary_dev);
5a5f2e3e Sridhar
Re: [PATCH 2/2] bpf: btf: remove a couple conditions
On Fri, Apr 27, 2018 at 10:21:17PM +0200, Daniel Borkmann wrote: > On 04/27/2018 09:39 PM, Dan Carpenter wrote: > > On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote: > >> On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote: > >>> On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote: > >>>> We know "err" is zero so we can remove these and pull the code in one > >>>> indent level. > >>>> > >>>> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com> > >>> Thanks for the simplification! > >>> > >>> Acked-by: Martin KaFai Lau <ka...@fb.com> > >> btw, it should be for bpf-next. Please tag the subject with bpf-next when > >> you respin. Thanks! > > Dan, thanks a lot for your fixes! Please respin with addressing Martin's > feedback when you get a chance. > My understanding is that he'd prefer we just ignore the static checker warning since it's a false positive. Should I instead initialize the size to zero or something just to silence it? regards, dan carpenter
Re: [PATCH 2/2] bpf: btf: remove a couple conditions
On Fri, Apr 27, 2018 at 10:55:46AM -0700, Martin KaFai Lau wrote: > On Fri, Apr 27, 2018 at 10:20:25AM -0700, Martin KaFai Lau wrote: > > On Fri, Apr 27, 2018 at 05:04:59PM +0300, Dan Carpenter wrote: > > > We know "err" is zero so we can remove these and pull the code in one > > > indent level. > > > > > > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com> > > Thanks for the simplification! > > > > Acked-by: Martin KaFai Lau <ka...@fb.com> > btw, it should be for bpf-next. Please tag the subject with bpf-next when > you respin. Thanks! > I'm working against linux-next. For networking, I have a separate tree which I use to figure out if it's in net or net-next. It's kind of a headache (but obviously networking is the largest subtree so it's required). Is there an automated way to tie a Fixes tag from linux-next to a subtree? regards, dan carpenter
[PATCH 1/2] bpf: btf: silence uninitialize variable warnings
Smatch complains that size can be uninitialized if btf_type_id_size()
returns NULL. It seems reasonable enough to check for that.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
This goes to the BPF tree (linux-next).
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 22e1046a1a86..e631b6fd60d3 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1229,7 +1229,8 @@ static int btf_array_check_member(struct btf_verifier_env
*env,
}
array_type_id = member->type;
- btf_type_id_size(btf, _type_id, _size);
+ if (!btf_type_id_size(btf, _type_id, _size))
+ return -EINVAL;
struct_size = struct_type->size;
bytes_offset = BITS_ROUNDDOWN_BYTES(struct_bits_off);
if (struct_size - bytes_offset < array_size) {
@@ -1351,6 +1352,8 @@ static void btf_array_seq_show(const struct btf *btf,
const struct btf_type *t,
elem_type_id = array->type;
elem_type = btf_type_id_size(btf, _type_id, _size);
+ if (!elem_type)
+ return;
elem_ops = btf_type_ops(elem_type);
seq_puts(m, "[");
for (i = 0; i < array->nelems; i++) {
[PATCH 2/2] bpf: btf: remove a couple conditions
We know "err" is zero so we can remove these and pull the code in one
indent level.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
This applies to the BPF tree (linux-next)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index e631b6fd60d3..7cb0905f37c2 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1973,16 +1973,14 @@ static struct btf *btf_parse(void __user *btf_data, u32
btf_data_size,
if (err)
goto errout;
- if (!err && log->level && bpf_verifier_log_full(log)) {
+ if (log->level && bpf_verifier_log_full(log)) {
err = -ENOSPC;
goto errout;
}
- if (!err) {
- btf_verifier_env_free(env);
- btf_get(btf);
- return btf;
- }
+ btf_verifier_env_free(env);
+ btf_get(btf);
+ return btf;
errout:
btf_verifier_env_free(env);
Re: [PATCH 03/39] proc: introduce proc_create_seq_private
On Thu, Apr 19, 2018 at 02:41:04PM +0200, Christoph Hellwig wrote:
> diff --git a/drivers/s390/cio/blacklist.c b/drivers/s390/cio/blacklist.c
> index 2a3f874a21d5..ad35cddcf6af 100644
> --- a/drivers/s390/cio/blacklist.c
> +++ b/drivers/s390/cio/blacklist.c
> @@ -391,28 +391,15 @@ static const struct seq_operations
> cio_ignore_proc_seq_ops = {
> .show = cio_ignore_proc_seq_show,
> };
>
> -static int
> -cio_ignore_proc_open(struct inode *inode, struct file *file)
> -{
> - return seq_open_private(file, _ignore_proc_seq_ops,
> - sizeof(struct ccwdev_iter));
> -}
> -
> -static const struct file_operations cio_ignore_proc_fops = {
> - .open= cio_ignore_proc_open,
> - .read= seq_read,
> - .llseek = seq_lseek,
> - .release = seq_release_private,
> - .write = cio_ignore_write,
The cio_ignore_write() function isn't used any more so compilers will
complain.
> -};
> -
> static int
> cio_ignore_proc_init (void)
> {
> struct proc_dir_entry *entry;
>
> - entry = proc_create("cio_ignore", S_IFREG | S_IRUGO | S_IWUSR, NULL,
> - _ignore_proc_fops);
> + entry = proc_create_seq_private("cio_ignore",
> + S_IFREG | S_IRUGO | S_IWUSR, NULL,
> + _ignore_proc_seq_ops, sizeof(struct ccwdev_iter),
> + NULL);
> if (!entry)
> return -ENOENT;
> return 0;
regards,
dan carpenter
[PATCH net] Revert "macsec: missing dev_put() on error in macsec_newlink()"
This patch is just wrong, sorry. I was trying to fix a static checker
warning and misread the code. The reference taken in macsec_newlink()
is released in macsec_free_netdev() when the netdevice is destroyed.
This reverts commit 5dcd8400884cc4a043a6d4617e042489e5d566a9.
Reported-by: Laura Abbott <labb...@redhat.com>
Fixes: 5dcd8400884c ("macsec: missing dev_put() on error in macsec_newlink()")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
Acked-by: Sabrina Dubroca <s...@queasysnail.net>
---
I sent this earlier but I messed up the CC list. I've updated the
commit message as well.
drivers/net/macsec.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 9cbb0c8a896a..7de88b33d5b9 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3277,7 +3277,7 @@ static int macsec_newlink(struct net *net, struct
net_device *dev,
err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
- goto put_dev;
+ goto unregister;
/* need to be already registered so that ->init has run and
* the MAC addr is set
@@ -3316,8 +3316,7 @@ static int macsec_newlink(struct net *net, struct
net_device *dev,
macsec_del_dev(macsec);
unlink:
netdev_upper_dev_unlink(real_dev, dev);
-put_dev:
- dev_put(real_dev);
+unregister:
unregister_netdevice(dev);
return err;
}
--
2.16.2
[PATCH] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
The skb_segment() function returns error pointers on error. It never
returns NULL.
Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index b2badf6b23cd..8e157806df7a 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6649,7 +6649,7 @@ static __init int test_skb_segment(void)
}
segs = skb_segment(skb, features);
- if (segs) {
+ if (!IS_ERR(segs)) {
kfree_skb_list(segs);
ret = 0;
pr_info("%s: success in skb_segment!", __func__);
[PATCH net-next] ibmvnic: Potential NULL dereference in clean_one_tx_pool()
There is an && vs || typo here, which potentially leads to a NULL
dereference.
Fixes: e9e1e97884b7 ("ibmvnic: Update TX pool cleaning routine")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
b/drivers/net/ethernet/ibm/ibmvnic.c
index 5632c030811b..0389a7a52152 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1135,7 +1135,7 @@ static void clean_one_tx_pool(struct ibmvnic_adapter
*adapter,
u64 tx_entries;
int i;
- if (!tx_pool && !tx_pool->tx_buff)
+ if (!tx_pool || !tx_pool->tx_buff)
return;
tx_entries = tx_pool->num_buffers;
[PATCH] macsec: missing dev_put() on error in macsec_newlink()
We moved the dev_hold(real_dev); call earlier in the function but forgot
to update the error paths.
Fixes: 0759e552bce7 ("macsec: fix negative refcnt on parent link")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 7de88b33d5b9..9cbb0c8a896a 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3277,7 +3277,7 @@ static int macsec_newlink(struct net *net, struct
net_device *dev,
err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
- goto unregister;
+ goto put_dev;
/* need to be already registered so that ->init has run and
* the MAC addr is set
@@ -3316,7 +3316,8 @@ static int macsec_newlink(struct net *net, struct
net_device *dev,
macsec_del_dev(macsec);
unlink:
netdev_upper_dev_unlink(real_dev, dev);
-unregister:
+put_dev:
+ dev_put(real_dev);
unregister_netdevice(dev);
return err;
}
[PATCH] ncpfs: memory corruption in ncp_read_kernel()
If the server is malicious then *bytes_read could be larger than the
size of the "target" buffer. It would lead to memory corruption when we
do the memcpy().
Reported-by: Dr Silvio Cesare of InfoSect
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/staging/ncpfs/ncplib_kernel.c
b/drivers/staging/ncpfs/ncplib_kernel.c
index 804adfebba2f..3e047eb4cc7c 100644
--- a/drivers/staging/ncpfs/ncplib_kernel.c
+++ b/drivers/staging/ncpfs/ncplib_kernel.c
@@ -981,6 +981,10 @@ ncp_read_kernel(struct ncp_server *server, const char
*file_id,
goto out;
}
*bytes_read = ncp_reply_be16(server, 0);
+ if (*bytes_read > to_read) {
+ result = -EINVAL;
+ goto out;
+ }
source = ncp_reply_data(server, 2 + (offset & 1));
memcpy(target, source, *bytes_read);
Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver
On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote: > On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote: > > This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs > > with DPAA2 (DataPath Acceleration Architecture v2). The driver manages > > switch objects discovered on the fsl-mc bus. A description of the driver > > can be found in the associated README file. > > Hi Greg > > This code has much better quality than the usual stuff in staging. I > see no reason not to merge it. Yeah. It seems pretty decent. Stuart, Laurentiu, care to comment? Meanwhile, netdev and DaveM aren't even on the CC list and they're the ones to ultimately decide. regards, dan carpenter
Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send
On Tue, Mar 13, 2018 at 08:06:50PM +0100, Mohammed Gamal wrote:
> @@ -791,6 +791,7 @@ static inline int netvsc_send_pkt(
>
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> }
>
> + ring_avail = hv_ringbuf_avail_percent(_channel->outbound);
> if (ret == 0) {
> atomic_inc_return(>queue_sends);
>
Could you move the assignment inside the "ret == 0" path closer to where
it's used?
regards,
dan carpenter
[PATCH net] qed: Use after free in qed_rdma_free()
We're dereferencing "p_hwfn->p_rdma_info" but that is freed on the line
before in qed_rdma_resc_free(p_hwfn).
Fixes: 9de506a547c0 ("qed: Free RoCE ILT Memory on rmmod qedr")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
index f3ee6538b553..a411f9c702a1 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_rdma.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_rdma.c
@@ -379,8 +379,8 @@ static void qed_rdma_free(struct qed_hwfn *p_hwfn)
DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Freeing RDMA\n");
qed_rdma_free_reserved_lkey(p_hwfn);
- qed_rdma_resc_free(p_hwfn);
qed_cxt_free_proto_ilt(p_hwfn, p_hwfn->p_rdma_info->proto);
+ qed_rdma_resc_free(p_hwfn);
}
static void qed_rdma_get_guid(struct qed_hwfn *p_hwfn, u8 *guid)
[PATCH 1/2 net-next] net/ncsi: use kfree_skb() instead of kfree()
We're supposed to use kfree_skb() to free these sk_buffs.
Fixes: 955dc68cb9b2 ("net/ncsi: Add generic netlink family")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index d4201665a580..b73239b76349 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -183,7 +183,7 @@ static int ncsi_pkg_info_nl(struct sk_buff *msg, struct
genl_info *info)
hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
_genl_family, 0, NCSI_CMD_PKG_INFO);
if (!hdr) {
- kfree(skb);
+ kfree_skb(skb);
return -EMSGSIZE;
}
@@ -204,7 +204,7 @@ static int ncsi_pkg_info_nl(struct sk_buff *msg, struct
genl_info *info)
err:
genlmsg_cancel(skb, hdr);
- kfree(skb);
+ kfree_skb(skb);
return rc;
}
[PATCH 2/2 net-next] net/ncsi: unlock on error in ncsi_set_interface_nl()
There are two error paths which are missing unlocks in this function.
Fixes: 955dc68cb9b2 ("net/ncsi: Add generic netlink family")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index b73239b76349..05fcfb4fbe1d 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -299,6 +299,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg,
struct genl_info *info)
package = np;
if (!package) {
/* The user has set a package that does not exist */
+ spin_unlock_irqrestore(>lock, flags);
return -ERANGE;
}
@@ -317,6 +318,7 @@ static int ncsi_set_interface_nl(struct sk_buff *msg,
struct genl_info *info)
/* The user has set a channel that does not exist on this
* package
*/
+ spin_unlock_irqrestore(>lock, flags);
netdev_info(ndp->ndev.dev, "NCSI: Channel %u does not exist!\n",
channel_id);
return -ERANGE;
[PATCH net] net: aquantia: Fix error handling in aq_pci_probe()
We should check "self->aq_hw" for allocation failure, and also we should
free it on the error paths.
Fixes: 23ee07ad3c2f ("net: aquantia: Cleanup pci functions module")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index 22889fc158f2..87c4308b52a7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -226,6 +226,10 @@ static int aq_pci_probe(struct pci_dev *pdev,
goto err_ioremap;
self->aq_hw = kzalloc(sizeof(*self->aq_hw), GFP_KERNEL);
+ if (!self->aq_hw) {
+ err = -ENOMEM;
+ goto err_ioremap;
+ }
self->aq_hw->aq_nic_cfg = aq_nic_get_cfg(self);
for (bar = 0; bar < 4; ++bar) {
@@ -235,19 +239,19 @@ static int aq_pci_probe(struct pci_dev *pdev,
mmio_pa = pci_resource_start(pdev, bar);
if (mmio_pa == 0U) {
err = -EIO;
- goto err_ioremap;
+ goto err_free_aq_hw;
}
reg_sz = pci_resource_len(pdev, bar);
if ((reg_sz <= 24 /*ATL_REGS_SIZE*/)) {
err = -EIO;
- goto err_ioremap;
+ goto err_free_aq_hw;
}
self->aq_hw->mmio = ioremap_nocache(mmio_pa, reg_sz);
if (!self->aq_hw->mmio) {
err = -EIO;
- goto err_ioremap;
+ goto err_free_aq_hw;
}
break;
}
@@ -255,7 +259,7 @@ static int aq_pci_probe(struct pci_dev *pdev,
if (bar == 4) {
err = -EIO;
- goto err_ioremap;
+ goto err_free_aq_hw;
}
numvecs = min((u8)AQ_CFG_VECS_DEF,
@@ -290,6 +294,8 @@ static int aq_pci_probe(struct pci_dev *pdev,
aq_pci_free_irq_vectors(self);
err_hwinit:
iounmap(self->aq_hw->mmio);
+err_free_aq_hw:
+ kfree(self->aq_hw);
err_ioremap:
free_netdev(ndev);
err_pci_func:
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
That found 4 that I think Wolfram's grep missed. arch/um/drivers/vector_user.h |2 -- drivers/gpu/drm/mxsfb/mxsfb_regs.h|2 -- drivers/video/fbdev/mxsfb.c |2 -- include/drm/drm_scdc_helper.h |3 --- But it didn't find the two bugs that Geert found where the right side wasn't a number literal. drivers/net/can/m_can/m_can.c:#define RXFC_FWM_MASK (0x7f < RXFC_FWM_SHIFT) drivers/usb/gadget/udc/goku_udc.h:#define INT_EPnNAK(n) (0x00100 < (n)) /* 0 < n < 4 */ regards, dan carpenter
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Tue, Feb 06, 2018 at 02:15:51PM +0100, Julia Lawall wrote:
>
>
> On Tue, 6 Feb 2018, Dan Carpenter wrote:
>
> > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> > > In one Renesas driver, I found a typo which turned an intended bit shift
> > > ('<<')
> > > into a comparison ('<'). Because this is a subtle issue, I looked tree
> > > wide for
> > > similar patterns. This small patch series is the outcome.
> > >
> > > Buildbot and checkpatch are happy. Only compile-tested. To be applied
> > > individually per sub-system, I think. I'd think only the net: amd: patch
> > > needs
> > > to be conisdered for stable, but I leave this to people who actually know
> > > this
> > > driver.
> > >
> > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup,
> > > only
> > > cppcheck reported a 'coding style' issue with a low prio.
> > >
> >
> > Most of these are inside macros so it makes it complicated for Smatch
> > to warn about them. It might be easier in Coccinelle. Julia the bugs
> > look like this:
> >
> > - reissue_mask |= 0x < 4;
> > + reissue_mask |= 0x << 4;
>
> Thanks. I'll take a look. Do you have an example of the macro issue
> handy?
>
It's the same:
#define EXYNOS_CIIMGEFF_PAT_CBCR_MASK ((0xff < 13) | (0xff < 0))
Smatch only sees the outside of the macro (where it is used in the code)
and the pre-processed code.
regards,
dan carpenter
Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask
On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> In one Renesas driver, I found a typo which turned an intended bit shift
> ('<<')
> into a comparison ('<'). Because this is a subtle issue, I looked tree wide
> for
> similar patterns. This small patch series is the outcome.
>
> Buildbot and checkpatch are happy. Only compile-tested. To be applied
> individually per sub-system, I think. I'd think only the net: amd: patch needs
> to be conisdered for stable, but I leave this to people who actually know this
> driver.
>
> CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup, only
> cppcheck reported a 'coding style' issue with a low prio.
>
Most of these are inside macros so it makes it complicated for Smatch
to warn about them. It might be easier in Coccinelle. Julia the bugs
look like this:
- reissue_mask |= 0x < 4;
+ reissue_mask |= 0x << 4;
regards,
dan carpenter
> Wolfram Sang (4):
> v4l: vsp1: fix mask creation for MULT_ALPHA_RATIO
> drm/exynos: fix comparison to bitshift when dealing with a mask
> v4l: dvb-frontends: stb0899: fix comparison to bitshift when dealing
> with a mask
> net: amd-xgbe: fix comparison to bitshift when dealing with a mask
>
> drivers/gpu/drm/exynos/regs-fimc.h| 2 +-
> drivers/media/dvb-frontends/stb0899_reg.h | 8
> drivers/media/platform/vsp1/vsp1_regs.h | 2 +-
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> --
> 2.11.0
Re: [PATCH] wireless: zd1211rw: remove redundant assignment of pointer 'q'
On Wed, Jan 31, 2018 at 02:58:57PM +0200, Andy Shevchenko wrote: > On Tue, Jan 30, 2018 at 8:25 PM, Colin King <colin.k...@canonical.com> wrote: > > From: Colin Ian King <colin.k...@canonical.com> > > > > Pointer q is initialized and then almost immediately afterwards being > > re-assigned the same value. Remove the second redundant assignment. > > > > Don't you see strange that in the same context of the patch two users > of q are present? > > How did you test this? > The patch is obviously correct, I don't understand the question. regards, dan carpenter
[PATCH v2] tools/bpftool: silence a static checker warning
There is a static checker warning that "proglen" has an upper bound but
no lower bound. The allocation will just fail harmlessly so it's not a
big deal.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: change the type to unsigned instead of checking for negatives
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index 30044bc4f389..58c2bab4ef6e 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -172,7 +172,8 @@ static uint8_t *get_last_jit_image(char *haystack, size_t
hlen,
{
char *ptr, *pptr, *tmp;
off_t off = 0;
- int ret, flen, proglen, pass, ulen = 0;
+ unsigned int proglen;
+ int ret, flen, pass, ulen = 0;
regmatch_t pmatch[1];
unsigned long base;
regex_t regex;
@@ -199,7 +200,7 @@ static uint8_t *get_last_jit_image(char *haystack, size_t
hlen,
}
ptr = haystack + off - (pmatch[0].rm_eo - pmatch[0].rm_so);
- ret = sscanf(ptr, "flen=%d proglen=%d pass=%d image=%lx",
+ ret = sscanf(ptr, "flen=%d proglen=%u pass=%d image=%lx",
, , , );
if (ret != 4) {
regfree();
@@ -239,7 +240,7 @@ static uint8_t *get_last_jit_image(char *haystack, size_t
hlen,
}
assert(ulen == proglen);
- printf("%d bytes emitted from JIT compiler (pass:%d, flen:%d)\n",
+ printf("%u bytes emitted from JIT compiler (pass:%d, flen:%d)\n",
proglen, pass, flen);
printf("%lx + :\n", base);
Re: [PATCH net-next] bnxt_en: uninitialized variable in bnxt_rx_pkt()
Never mind. Colin already sent this one. regards, dan carpenter
[PATCH net-next] bnxt_en: uninitialized variable in bnxt_rx_pkt()
There are a couple failure paths where "len" is used uninitialized. It
means we record the number of rx_packets incorrectly.
Fixes: 6a8788f25625 ("bnxt_en: add support for software dynamic interrupt
moderation")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index cf6ebf1e324b..5b5c4f266f1b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -1482,7 +1482,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi
*bnapi, u32 *raw_cons,
u32 tmp_raw_cons = *raw_cons;
u16 cfa_code, cons, prod, cp_cons = RING_CMP(tmp_raw_cons);
struct bnxt_sw_rx_bd *rx_buf;
- unsigned int len;
+ unsigned int len = 0;
u8 *data_ptr, agg_bufs, cmp_type;
dma_addr_t dma_addr;
struct sk_buff *skb;
[PATCH] tools/bpftool: silence a static check warning
There is a static checker warning that proglen has an upper bound but no
lower bound. The allocation will just fail harmlessly so it's not a big
deal.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index 30044bc4f389..2d7bb5dc0b8c 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -205,7 +205,7 @@ static uint8_t *get_last_jit_image(char *haystack, size_t
hlen,
regfree();
return NULL;
}
- if (proglen > 100) {
+ if (proglen < 0 || proglen > 100) {
printf("proglen of %d too big, stopping\n", proglen);
return NULL;
}
Re: [PATCH net 3/3] hv_netvsc: Fix the default receive buffer size
On Thu, Dec 07, 2017 at 04:10:55PM -0800, Stephen Hemminger wrote:
> From: Haiyang Zhang <haiya...@microsoft.com>
>
> The intended size is 16 MB, and the default slot size is 1728.
> So, NETVSC_DEFAULT_RX should be 16*1024*1024 / 1728 = 9709.
>
> Fixes: 5023a6db73196 ("netvsc: increase default receive buffer size")
> Signed-off-by: Haiyang Zhang <haiya...@microsoft.com>
> Signed-off-by: Stephen Hemminger <sthem...@microsoft.com>
> ---
> drivers/net/hyperv/netvsc_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index dc70de674ca9..edfcde5d3621 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -50,7 +50,7 @@
> #define NETVSC_MIN_TX_SECTIONS 10
> #define NETVSC_DEFAULT_TX192 /* ~1M */
> #define NETVSC_MIN_RX_SECTIONS 10 /* ~64K */
> -#define NETVSC_DEFAULT_RX10485 /* Max ~16M */
> +#define NETVSC_DEFAULT_RX9709/* ~16M */
How does this bug look like to the user? Memory corruption?
It's weird to me reviewing this code that the default sizes are
stored in netvsc_drv.c and the max sizes are stored in hyperv_net.h.
Could we move these to hyperv_net.h? We could write it like:
#define NETVSC_DEFAULT_RX ((16 * 1024 * 1024) / NETVSC_RECV_SECTION_SIZE)
16MB is sort of a weird default because it's larger than the 15MB
allowed for legacy versions, but it's smaller than the 32MB you'd want
for the current versions.
regards,
dan carpenter
Re: [PATCH net 1/3] hv_netvsc: Correct the max receive buffer size
On Thu, Dec 07, 2017 at 04:10:53PM -0800, Stephen Hemminger wrote: > From: Haiyang Zhang <haiya...@microsoft.com> > > It should be 31 MB on recent host versions. > > Signed-off-by: Haiyang Zhang <haiya...@microsoft.com> > Signed-off-by: Stephen Hemminger <sthem...@microsoft.com> This is very vague. What does "recent" mean in this context? There are also some unrelated white space changes here which make the patch harder to read. This patch kind of makes the bug fixed by patch 2 even worse because before the receive buffer was capped at around 16MB and now we can set the receive buffer to 31MB. It might make sense to fold the two patches together. Is patch 2 a memory corruption bug? The changelog doesn't really say what the user visible effects of the bug are. Basically if you make the buffer too small then it's a performance issue but if you make it too large what happens? It's not clear to me. regards, dan carpenter
Re: [PATCH] bnxt_en: Uninitialized variable in bnxt_tc_parse_actions()
I'm sorry, I left out that this applies to net-next. regards, dan carpenter
[PATCH] bnxt_en: Uninitialized variable in bnxt_tc_parse_actions()
Smatch warns that:
drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c:160 bnxt_tc_parse_actions()
error: uninitialized symbol 'rc'.
"rc" is either uninitialized or set to zero here so we can just remove
the check.
Fixes: 8c95f773b4a3 ("bnxt_en: add support for Flower based vxlan encap/decap
offload")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index d5031f436f83..125b146da031 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -157,9 +157,6 @@ static int bnxt_tc_parse_actions(struct bnxt *bp,
}
}
- if (rc)
- return rc;
-
/* Tunnel encap/decap action must be accompanied by a redirect action */
if ((actions->flags & BNXT_TC_ACTION_FLAG_TUNNEL_ENCAP ||
actions->flags & BNXT_TC_ACTION_FLAG_TUNNEL_DECAP) &&
@@ -169,7 +166,7 @@ static int bnxt_tc_parse_actions(struct bnxt *bp,
return -EINVAL;
}
- return rc;
+ return 0;
}
#define GET_KEY(flow_cmd, key_type)\
Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
On Wed, Nov 29, 2017 at 09:37:15PM +0530, Souptick Joarder wrote:
> >> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv,
> >> struct port_status status)
> >> +{
> >> + u64 data;
> >> + u64 prtx;
> >> + u64 miscx;
> >> + int timeout;
> >> +
>
> >> +
> >> + switch (status.speed) {
> >> + case 10:
> >
> > In my opinion, instead of hard coding the value, is it fine to use ENUM ?
>Similar comments applicable in other places where hard coded values are
> used.
>
10 means 10M right? That's not really a magic number. It's fine.
> >> +static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
> >> +{
>
> >> +
> >> + if (use_ber) {
> >> + timeout = 1;
> >> + do {
> >> + data =
> >> +
> >> oct_csr_read(BGX_SPU_BR_STATUS1(priv->node, priv->bgx, priv->index));
> >> + if (data & BIT(0))
> >> + break;
> >> + timeout--;
> >> + udelay(1);
> >> + } while (timeout);
> >
> > In my opinion, it's better to implement similar kind of loops inside macros.
I don't understand what you mean here. For what it's worth this code
seems clear enough to me (except for the bad indenting of oct_csr_read().
It should be something like:
data =
oct_csr_read(BGX_SPU_BR_STATUS1(priv->node,
priv->bgx, priv->index));
That's over the 80 char limit but so is the original code.
regards,
dan carpenter
[PATCH net-next] liquidio: Missing error code in liquidio_init_nic_module()
We accidentally return success if lio_vf_rep_modinit() fails instead of
propogating the error code.
Fixes: e20f469660ad ("liquidio: synchronize VF representor names with NIC
firmware")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/cavium/liquidio/lio_main.c
b/drivers/net/ethernet/cavium/liquidio/lio_main.c
index f05045a69dcc..6aa0eee88ea5 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_main.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_main.c
@@ -4038,7 +4038,8 @@ static int liquidio_init_nic_module(struct octeon_device
*oct)
*/
if (!oct->octeon_id &&
oct->fw_info.app_cap_flags & LIQUIDIO_SWITCHDEV_CAP) {
- if (lio_vf_rep_modinit()) {
+ retval = lio_vf_rep_modinit();
+ if (retval) {
liquidio_stop_nic_module(oct);
goto octnet_init_failure;
}
[PATCH net-next] xdp: sample: Missing curly braces in read_route()
The assert statement is supposed to be part of the else branch but the
curly braces were accidentally left off.
Fixes: 3e29cd0e6563 ("xdp: Sample xdp program implementing ip forward")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/samples/bpf/xdp_router_ipv4_user.c
b/samples/bpf/xdp_router_ipv4_user.c
index 2c1fe3f4b1a4..916462112d55 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -206,12 +206,13 @@ static void read_route(struct nlmsghdr *nh, int nll)
direct_entry.arp.mac = 0;
direct_entry.arp.dst = 0;
if (route.dst_len == 32) {
- if (nh->nlmsg_type == RTM_DELROUTE)
+ if (nh->nlmsg_type == RTM_DELROUTE) {
assert(bpf_map_delete_elem(map_fd[3],
) == 0);
- else
+ } else {
if (bpf_map_lookup_elem(map_fd[2],
, _entry.arp.mac) == 0)
direct_entry.arp.dst =
route.dst;
assert(bpf_map_update_elem(map_fd[3],
, _entry, 0) == 0);
+ }
}
for (i = 0; i < 4; i++)
prefix_key->data[i] = (route.dst >> i * 8) &
0xff;
[PATCH net-next] bnxt: delete some unreachable code
We return on the previous line so this "return 0;" statement should just be deleted. Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c index b6aa7db99705..69186d188c43 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c @@ -148,7 +148,6 @@ static int bnxt_vf_rep_setup_tc_block(struct net_device *dev, return tcf_block_cb_register(f->block, bnxt_vf_rep_setup_tc_block_cb, vf_rep, vf_rep); - return 0; case TC_BLOCK_UNBIND: tcf_block_cb_unregister(f->block, bnxt_vf_rep_setup_tc_block_cb, vf_rep);
Re: [PATCH net-next] qed: Set error code for allocation failures
On Fri, Oct 27, 2017 at 05:32:42PM +0800, Yunsheng Lin wrote:
> Hi, Dan
>
> On 2017/10/27 14:40, Dan Carpenter wrote:
> > There are several places where we accidentally return success when
> > kcalloc() fails.
> >
> > Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and
> > processing mpa fpdus")
> > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> >
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > index 409041eab189..6366f2ef82b7 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
> > @@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> > struct qed_ll2_cbs cbs;
> > u32 mpa_buff_size;
> > u16 n_ooo_bufs;
> > - int rc = 0;
> > + int rc;
> > int i;
> >
> > iwarp_info = _hwfn->p_rdma_info->iwarp;
> > @@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
> > if (rc)
> > goto err;
> >
> > + rc = -ENOMEM;
> > iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
> > sizeof(*iwarp_info->partial_fpdus),
> > GFP_KERNEL);
>
> Does the memory allocated here need to be freed when error happens below?
>
Hm... I think you're right that it leaks. Also I'm confused by the
qed_iwarp_ll2_alloc_buffers() allocation. The comment in there says
that /* buffers will be deallocated by qed_ll2 */ but qed_ll2 is not
a function name or something which is useful to grep.
regards,
dan carpenter
[PATCH net-next] qed: Set error code for allocation failures
There are several places where we accidentally return success when
kcalloc() fails.
Fixes: fcb39f6c10b2 ("qed: Add mpa buffer descriptors for storing and
processing mpa fpdus")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
index 409041eab189..6366f2ef82b7 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iwarp.c
@@ -2585,7 +2585,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
struct qed_ll2_cbs cbs;
u32 mpa_buff_size;
u16 n_ooo_bufs;
- int rc = 0;
+ int rc;
int i;
iwarp_info = _hwfn->p_rdma_info->iwarp;
@@ -2696,6 +2696,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
if (rc)
goto err;
+ rc = -ENOMEM;
iwarp_info->partial_fpdus = kcalloc((u16)p_hwfn->p_rdma_info->num_qps,
sizeof(*iwarp_info->partial_fpdus),
GFP_KERNEL);
@@ -2724,7 +2725,7 @@ qed_iwarp_ll2_start(struct qed_hwfn *p_hwfn,
for (i = 0; i < data.input.rx_num_desc; i++)
list_add_tail(_info->mpa_bufs[i].list_entry,
_info->mpa_buf_list);
- return rc;
+ return 0;
err:
qed_iwarp_ll2_stop(p_hwfn, p_ptt);
Re: [REGRESSION, BISECTED] Broken networking with net/phy/marvell
You have a "Marvell 88E1116R" device, right? We used to set MII_88E1121_PHY_MSCR_RX_DELAY and MII_88E1121_PHY_MSCR_TX_DELAY bits unconditionally but now it depends on phydev->interface. I don't know the code/hardware well enough to say what phydev->interface is for you. regards, dan carpenter
Re: [REGRESSION, BISECTED] Broken networking with net/phy/marvell
On Thu, Oct 26, 2017 at 03:28:36PM +0300, Aaro Koskinen wrote:
> Hi,
>
> When upgrading from v4.13 to v4.14-rc6 on OpenRD Client, the box loses
> network connectivity.
>
> Bisection points to:
>
> commit 5987feb38aa55e035ce5376c02aba88a604cc881
> Author: Dan Carpenter <dan.carpen...@oracle.com>
> Date: Fri Aug 4 11:17:21 2017 +0300
>
> net: phy: marvell: logical vs bitwise OR typo
>
> However, it seems this commit just unhides another issue in the original
> commit 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay
> configuration"): when we are configuring the MSCR delay bits, we are
> probably clearing the bits with a wrong mask (i.e. we might be disabling
> something else not intended)...
>
> I have tested the below change and it seems to fix the networking. Any
> comments?
>
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 15cbcdb..500d7c1 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -474,7 +474,7 @@ static int m88e1121_config_aneg_rgmii_delays(struct
> phy_device *phydev)
> goto out;
> }
>
> - mscr &= MII_88E1121_PHY_MSCR_DELAY_MASK;
> + mscr &= ~MII_88E1121_PHY_MSCR_DELAY_MASK;
>
I don't think your fix is right. That mask was like that in the
original m88e1121_config_aneg() code before commit 864dc729d528 ("net:
phy: marvell: Refactor m88e1121 RGMII delay configuration"). Perhaps
the bug is that m88e1116r_config_init() used to not have the mask and
now it does?
Another difference I see is that we also didn't used to call
marvell_set_page(phydev, oldpage); on error, but I think that's a bugfix
and not related to what you're seeing.
I don't know the code well enough to write a fix.
regards,
dan carpenter
[PATCH net-next] tipc: checking for NULL instead of IS_ERR()
The tipc_alloc_conn() function never returns NULL, it returns error
pointers, so I have fixed the check.
Fixes: 14c04493cb77 ("tipc: add ability to order and receive topology events in
driver")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/net/tipc/server.c b/net/tipc/server.c
index 713077536d0c..acaef80fb88c 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -504,7 +504,7 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32 port, u32
type,
*(u32 *)_handle = port;
con = tipc_alloc_conn(tipc_topsrv(net));
- if (!con)
+ if (IS_ERR(con))
return false;
*conid = con->conid;
[PATCH 2/2 net-next] thunderbolt: Right shifting to zero bug in tbnet_handle_packet()
There is a problem when we do:
sequence = pkg->hdr.length_sn & TBIP_HDR_SN_MASK;
sequence >>= TBIP_HDR_SN_SHIFT;
TBIP_HDR_SN_SHIFT is 27, and right shifting a u8 27 bits is always
going to result in zero. The fix is to declare these variables as u32.
Fixes: e69b6c02b4c3 ("net: Add support for networking over Thunderbolt cable")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index 1a7bc0bf4598..435854688a7a 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -394,7 +394,7 @@ static int tbnet_handle_packet(const void *buf, size_t
size, void *data)
struct tbnet *net = data;
u32 command_id;
int ret = 0;
- u8 sequence;
+ u32 sequence;
u64 route;
/* Make sure the packet is for us */
[PATCH 1/2 v2 net-next] thunderbolt: Fix a couple right shifting to zero bugs
The problematic code looks like this:
res_seq = res_hdr->xd_hdr.length_sn & TB_XDOMAIN_SN_MASK;
res_seq >>= TB_XDOMAIN_SN_SHIFT;
TB_XDOMAIN_SN_SHIFT is 27, and right shifting a u8 27 bits is always
going to result in zero. The fix is to declare these variables as u32.
Fixes: d1ff70241a27 ("thunderbolt: Add support for XDomain discovery protocol")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: I accidentally sent this through the wrong list, so I'm resending to
netdev. Also Mika asked me to split it up because the Fixes tags
are different for these patches.
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 138027537d29..ff8d91189e99 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -56,7 +56,7 @@ static bool tb_xdomain_match(const struct tb_cfg_request *req,
case TB_CFG_PKG_XDOMAIN_RESP: {
const struct tb_xdp_header *res_hdr = pkg->buffer;
const struct tb_xdp_header *req_hdr = req->request;
- u8 req_seq, res_seq;
+ u32 req_seq, res_seq;
if (pkg->frame.size < req->response_size / 4)
return false;
@@ -476,7 +476,7 @@ static void tb_xdp_handle_request(struct work_struct *work)
struct tb_ctl *ctl = tb->ctl;
const uuid_t *uuid;
int ret = 0;
- u8 sequence;
+ u32 sequence;
u64 route;
route = ((u64)xhdr->route_hi << 32 | xhdr->route_lo) & ~BIT_ULL(63);
[PATCH net] selftests/net: rxtimestamp: Fix an off by one
The > should be >= so that we don't write one element beyond the end of
the array.
Fixes: 16e781224198 ("selftests/net: Add a test to validate behavior of rx
timestamps")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/tools/testing/selftests/networking/timestamping/rxtimestamp.c
b/tools/testing/selftests/networking/timestamping/rxtimestamp.c
index 00f286661dcd..dd4162fc0419 100644
--- a/tools/testing/selftests/networking/timestamping/rxtimestamp.c
+++ b/tools/testing/selftests/networking/timestamping/rxtimestamp.c
@@ -341,7 +341,7 @@ int main(int argc, char **argv)
return 0;
case 'n':
t = atoi(optarg);
- if (t > ARRAY_SIZE(test_cases))
+ if (t >= ARRAY_SIZE(test_cases))
error(1, 0, "Invalid test case: %d", t);
all_tests = false;
test_cases[t].enabled = true;
[PATCH 2/2 net-next] mlxsw: spectrum: Add missing error code on allocation failure
We accidentally return success if the kmalloc_array() call fails.
Fixes: 0e14cacb ("mlxsw: spectrum: Add the multicast routing hardware
logic")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
index 5e4ccbf17e3d..839eadf0765b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
@@ -763,8 +763,10 @@ mlxsw_sp_mr_tcam_region_init(struct mlxsw_sp *mlxsw_sp,
parman_prios = kmalloc_array(MLXSW_SP_MR_ROUTE_PRIO_MAX + 1,
sizeof(*parman_prios), GFP_KERNEL);
- if (!parman_prios)
+ if (!parman_prios) {
+ err = -ENOMEM;
goto err_parman_prios_alloc;
+ }
mr_tcam_region->parman_prios = parman_prios;
for (i = 0; i < MLXSW_SP_MR_ROUTE_PRIO_MAX + 1; i++)
[PATCH 1/2 net-next] mlxsw: spectrum: Fix check for IS_ERR() instead of NULL
mlxsw_afa_block_create() doesn't return error pointers, it returns NULL
on error.
Fixes: 0e14cacb ("mlxsw: spectrum: Add the multicast routing hardware
logic")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
index cda9e9ad10e3..5e4ccbf17e3d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_mr_tcam.c
@@ -239,8 +239,8 @@ mlxsw_sp_mr_tcam_afa_block_create(struct mlxsw_sp *mlxsw_sp,
int err;
afa_block = mlxsw_afa_block_create(mlxsw_sp->afa);
- if (IS_ERR(afa_block))
- return afa_block;
+ if (!afa_block)
+ return ERR_PTR(-ENOMEM);
err = mlxsw_afa_block_append_counter(afa_block, counter_index);
if (err)
[PATCH net v2] sctp: Fix a big endian bug in sctp_diag_dump()
The sctp_for_each_transport() function takes an pointer to int. The
cb->args[] array holds longs so it's only using the high 32 bits. It
works on little endian system but will break on big endian 64 bit
machines.
Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: The v1 patch changed the function to take a long pointer, but v2
just changes the caller.
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index 22ed01a76b19..a72a7d925d46 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -463,6 +463,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct
netlink_callback *cb,
.r = r,
.net_admin = netlink_net_capable(cb->skb, CAP_NET_ADMIN),
};
+ int pos = cb->args[2];
/* eps hashtable dumps
* args:
@@ -493,7 +494,8 @@ static void sctp_diag_dump(struct sk_buff *skb, struct
netlink_callback *cb,
goto done;
sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
- net, (int *)>args[2], );
+ net, , );
+ cb->args[2] = pos;
done:
cb->args[1] = cb->args[4];
[PATCH net] sctp: Fix a big endian bug in sctp_for_each_transport()
Fundamentally, the "pos" pointer points to "cb->args[2]" which is a long.
In the current code, we only use the high 32 bits and cast it as an
int. That works on little endian systems but will fail on big endian
systems.
Fixes: d25adbeb0cdb ("sctp: fix an use-after-free issue in sctp_sock_dump")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index d7d8cba01469..7d87439f299a 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -121,14 +121,14 @@ void sctp_transport_walk_stop(struct rhashtable_iter
*iter);
struct sctp_transport *sctp_transport_get_next(struct net *net,
struct rhashtable_iter *iter);
struct sctp_transport *sctp_transport_get_idx(struct net *net,
- struct rhashtable_iter *iter, int pos);
+ struct rhashtable_iter *iter, long pos);
int sctp_transport_lookup_process(int (*cb)(struct sctp_transport *, void *),
struct net *net,
const union sctp_addr *laddr,
const union sctp_addr *paddr, void *p);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
int (*cb_done)(struct sctp_transport *, void *),
- struct net *net, int *pos, void *p);
+ struct net *net, long *pos, void *p);
int sctp_for_each_endpoint(int (*cb)(struct sctp_endpoint *, void *), void *p);
int sctp_get_sctp_info(struct sock *sk, struct sctp_association *asoc,
struct sctp_info *info);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d4730ada7f32..0222743b3aa8 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4603,7 +4603,7 @@ struct sctp_transport *sctp_transport_get_next(struct net
*net,
struct sctp_transport *sctp_transport_get_idx(struct net *net,
struct rhashtable_iter *iter,
- int pos)
+ long pos)
{
void *obj = SEQ_START_TOKEN;
@@ -4659,7 +4659,7 @@ EXPORT_SYMBOL_GPL(sctp_transport_lookup_process);
int sctp_for_each_transport(int (*cb)(struct sctp_transport *, void *),
int (*cb_done)(struct sctp_transport *, void *),
- struct net *net, int *pos, void *p) {
+ struct net *net, long *pos, void *p) {
struct rhashtable_iter hti;
struct sctp_transport *tsp;
int ret;
diff --git a/net/sctp/sctp_diag.c b/net/sctp/sctp_diag.c
index 22ed01a76b19..e9d5405aa6ac 100644
--- a/net/sctp/sctp_diag.c
+++ b/net/sctp/sctp_diag.c
@@ -493,7 +493,7 @@ static void sctp_diag_dump(struct sk_buff *skb, struct
netlink_callback *cb,
goto done;
sctp_for_each_transport(sctp_sock_filter, sctp_sock_dump,
- net, (int *)>args[2], );
+ net, >args[2], );
done:
cb->args[1] = cb->args[4];
Re: [PATCH] e1000: avoid null pointer dereference on invalid stat type
On Thu, Sep 21, 2017 at 11:01:58PM +0100, Colin King wrote:
> @@ -1837,12 +1838,13 @@ static void e1000_get_ethtool_stats(struct net_device
> *netdev,
> p = (char *)adapter + stat->stat_offset;
> break;
> default:
> + p = NULL;
> WARN_ONCE(1, "Invalid E1000 stat type: %u index %d\n",
> stat->type, i);
> break;
> }
>
> - if (stat->sizeof_stat == sizeof(u64))
> + if (p && stat->sizeof_stat == sizeof(u64))
> data[i] = *(u64 *)p;
> else
> data[i] = *(u32 *)p;
The else side will still crash.
regards,
dan carpenter
[PATCH v2 net] sctp: potential read out of bounds in sctp_ulpevent_type_enabled()
This code causes a static checker warning because Smatch doesn't trust
anything that comes from skb->data. I've reviewed this code and I do
think skb->data can be controlled by the user here.
The sctp_event_subscribe struct has 13 __u8 fields and we want to see
if ours is non-zero. sn_type can be any value in the 0-USHRT_MAX range.
We're subtracting SCTP_SN_TYPE_BASE which is 1 << 15 so we could read
either before the start of the struct or after the end.
This is a very old bug and it's surprising that it would go undetected
for so long but my theory is that it just doesn't have a big impact so
it would be hard to notice.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: Use reverse-christmas-tree local variable ordering.
diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h
index 1060494ac230..b8c86ec1a8f5 100644
--- a/include/net/sctp/ulpevent.h
+++ b/include/net/sctp/ulpevent.h
@@ -153,8 +153,12 @@ __u16 sctp_ulpevent_get_notification_type(const struct
sctp_ulpevent *event);
static inline int sctp_ulpevent_type_enabled(__u16 sn_type,
struct sctp_event_subscribe *mask)
{
+ int offset = sn_type - SCTP_SN_TYPE_BASE;
char *amask = (char *) mask;
- return amask[sn_type - SCTP_SN_TYPE_BASE];
+
+ if (offset >= sizeof(struct sctp_event_subscribe))
+ return 0;
+ return amask[offset];
}
/* Given an event subscription, is this event enabled? */
[PATCH net] sctp: potential read out of bounds in sctp_ulpevent_type_enabled()
This code causes a static checker warning because Smatch doesn't trust
anything that comes from skb->data. I've reviewed this code and I do
think skb->data can be controlled by the user here.
The sctp_event_subscribe struct has 13 __u8 fields and we want to see
if ours is non-zero. sn_type can be any value in the 0-USHRT_MAX range.
We're subtracting SCTP_SN_TYPE_BASE which is 1 << 15 so we could read
either before the start of the struct or after the end.
This is a very old bug and it's surprising that it would go undetected
for so long but my theory is that it just doesn't have a big impact so
it would be hard to notice.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
I'm not terribly familiar with sctp and this is a static checker fix.
Please review it carefully.
diff --git a/include/net/sctp/ulpevent.h b/include/net/sctp/ulpevent.h
index 1060494ac230..e6873176bea7 100644
--- a/include/net/sctp/ulpevent.h
+++ b/include/net/sctp/ulpevent.h
@@ -154,7 +154,11 @@ static inline int sctp_ulpevent_type_enabled(__u16 sn_type,
struct sctp_event_subscribe *mask)
{
char *amask = (char *) mask;
- return amask[sn_type - SCTP_SN_TYPE_BASE];
+ int offset = sn_type - SCTP_SN_TYPE_BASE;
+
+ if (offset >= sizeof(struct sctp_event_subscribe))
+ return 0;
+ return amask[offset];
}
/* Given an event subscription, is this event enabled? */
[PATCH v2 net] net: qualcomm: rmnet: Fix a double free
There is a typo here so we accidentally free "skb" instead of "skbn".
It leads to a double free and a leak. After discussing with Subash,
it's better to just move the check before the allocation and avoid the
need to free.
Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial
implementation")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: Fix the leak as well. Thanks Subash!
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 557c9bf1a469..86b8c758f94e 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -84,6 +84,10 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
if (((int)skb->len - (int)packet_len) < 0)
return NULL;
+ /* Some hardware can send us empty frames. Catch them */
+ if (ntohs(maph->pkt_len) == 0)
+ return NULL;
+
skbn = alloc_skb(packet_len + RMNET_MAP_DEAGGR_SPACING, GFP_ATOMIC);
if (!skbn)
return NULL;
@@ -94,11 +98,5 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
memcpy(skbn->data, skb->data, packet_len);
skb_pull(skb, packet_len);
- /* Some hardware can send us empty frames. Catch them */
- if (ntohs(maph->pkt_len) == 0) {
- kfree_skb(skb);
- return NULL;
- }
-
return skbn;
}
[PATCH net] phy: mvebu-cp110: checking for NULL instead of IS_ERR()
devm_ioremap_resource() never returns NULL, it only returns error
pointers so this test needs to be changed.
Fixes: d0438bd6aa09 ("phy: add the mvebu cp110 comphy driver")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
This driver apparently is going through the net tree, but netdev isn't
listed as handling it in MAINTAINERS. Kishon, do you know what's up
with that?
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 73ebad6634a7..24578bd68ddc 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -576,8 +576,8 @@ static int mvebu_comphy_probe(struct platform_device *pdev)
return PTR_ERR(priv->regmap);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
priv->base = devm_ioremap_resource(>dev, res);
- if (!priv->base)
- return -ENOMEM;
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
for_each_available_child_of_node(pdev->dev.of_node, child) {
struct mvebu_comphy_lane *lane;
[PATCH net] net: qualcomm: rmnet: Fix a double free
This is called from rmnet_map_ingress_handler(). When the
rmnet_map_deaggregate() returns NULL then the caller calls
consume_skb(skb) which frees the skb. The kfree_skb() on this error
path leads to a double free.
Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial
implementation")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
This is from static analysis and not tested.
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 557c9bf1a469..0335fce54201 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -95,10 +95,8 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
skb_pull(skb, packet_len);
/* Some hardware can send us empty frames. Catch them */
- if (ntohs(maph->pkt_len) == 0) {
- kfree_skb(skb);
+ if (ntohs(maph->pkt_len) == 0)
return NULL;
- }
return skbn;
}
[PATCH net] nfp: double free on error in probe
Both the nfp_net_pf_app_start() and the nfp_net_pci_probe() functions
call nfp_net_pf_app_stop_ctrl(pf) so there is a double free. The free
should be done from the probe function because it's allocated there so
I have removed the call from nfp_net_pf_app_start().
Fixes: 02082701b974 ("nfp: create control vNICs and wire up rx/tx")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index d55f6c5ed1be..7c22cc4654b7 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -458,7 +458,7 @@ static int nfp_net_pf_app_start(struct nfp_pf *pf)
err = nfp_app_start(pf->app, pf->ctrl_vnic);
if (err)
- goto err_ctrl_stop;
+ return err;
if (pf->num_vfs) {
err = nfp_app_sriov_enable(pf->app, pf->num_vfs);
@@ -470,8 +470,6 @@ static int nfp_net_pf_app_start(struct nfp_pf *pf)
err_app_stop:
nfp_app_stop(pf->app);
-err_ctrl_stop:
- nfp_net_pf_app_stop_ctrl(pf);
return err;
}
Re: [PATCH] connector: Delete an error message for a failed memory allocation in cn_queue_alloc_callback_entry()
On Sun, Aug 27, 2017 at 11:16:06PM +, Waskiewicz Jr, Peter wrote: > On 8/27/17 3:26 PM, SF Markus Elfring wrote: > > From: Markus Elfring <elfr...@users.sourceforge.net> > > Date: Sun, 27 Aug 2017 21:18:37 +0200 > > > > Omit an extra message for a memory allocation failure in this function. > > > > This issue was detected by using the Coccinelle software. > > Did coccinelle trip on the message or the fact you weren't returning NULL? > You've misread the patch somehow. The existing code has a NULL return and it's preserved in Markus's patch. This sort of patch is to fix a checkpatch.pl warning. The error message from this kzalloc() isn't going to get printed because it's a small allocation and small allocations always succeed in current kernels. But probably the main reason checkpatch complains is that kmalloc() already prints a stack trace and a bunch of other information so the printk doesn't add anyting. Removing it saves a little memory. I'm mostly a fan of running checkpatch on new patches or staging and not on old code... regards, dan carpenter
[PATCH net-next] bpf: fix oops on allocation failure
"err" is set to zero if bpf_map_area_alloc() fails so it means we return
ERR_PTR(0) which is NULL. The caller, find_and_alloc_map(), is not
expecting NULL returns and will oops.
Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 78b2bb9370ac..a11b9f52ea4a 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -497,6 +497,7 @@ static struct bpf_map *sock_map_alloc(union bpf_attr *attr)
if (err)
goto free_stab;
+ err = -ENOMEM;
stab->sock_map = bpf_map_area_alloc(stab->map.max_entries *
sizeof(struct sock *),
stab->map.numa_node);
[PATCH] hinic: skb_pad() frees on error
The skb_pad() function frees the skb on error, so this code has a double
free.
Fixes: 00e57a6d4ad3 ("net-next/hinic: Add Tx operation")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index 5bf6a32faa46..abe3e38cd342 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -192,7 +192,7 @@ netdev_tx_t hinic_xmit_frame(struct sk_buff *skb, struct
net_device *netdev)
if (skb->len < MIN_SKB_LEN) {
if (skb_pad(skb, MIN_SKB_LEN - skb->len)) {
netdev_err(netdev, "Failed to pad skb\n");
- goto skb_error;
+ goto update_error_stats;
}
skb->len = MIN_SKB_LEN;
@@ -237,6 +237,7 @@ netdev_tx_t hinic_xmit_frame(struct sk_buff *skb, struct
net_device *netdev)
skb_error:
dev_kfree_skb_any(skb);
+update_error_stats:
u64_stats_update_begin(>txq_stats.syncp);
txq->txq_stats.tx_dropped++;
u64_stats_update_end(>txq_stats.syncp);
[PATCH net-next] hinic: uninitialized variable in hinic_api_cmd_init()
We never set the error code in this function.
Fixes: eabf0fad81d5 ("net-next/hinic: Initialize api cmd resources")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_api_cmd.c
b/drivers/net/ethernet/huawei/hinic/hinic_hw_api_cmd.c
index 8901801fe426..c40603a183df 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_api_cmd.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_api_cmd.c
@@ -941,6 +941,7 @@ int hinic_api_cmd_init(struct hinic_api_cmd_chain **chain,
if (IS_ERR(chain[chain_type])) {
dev_err(>dev, "Failed to create chain %d\n",
chain_type);
+ err = PTR_ERR(chain[chain_type]);
goto err_create_chain;
}
}
