[PATCH] net/netlabel: Add list_next_rcu() in rcu_dereference().

2017-11-16 Thread Tim Hansen 
Add list_next_rcu() for fetching next list in rcu_deference safely.

Found with sparse in linux-next tree on tag next-20171116.

Signed-off-by: Tim Hansen 
---
 net/netlabel/netlabel_addrlist.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netlabel/netlabel_addrlist.h b/net/netlabel/netlabel_addrlist.h
index d0f38bc..ac709f0 100644
--- a/net/netlabel/netlabel_addrlist.h
+++ b/net/netlabel/netlabel_addrlist.h
@@ -87,7 +87,7 @@ static inline struct netlbl_af4list 
*__af4list_valid_rcu(struct list_head *s,
struct list_head *i = s;
struct netlbl_af4list *n = __af4list_entry(s);
while (i != h && !n->valid) {
-   i = rcu_dereference(i->next);
+   i = rcu_dereference(list_next_rcu(i));
n = __af4list_entry(i);
}
return n;
@@ -154,7 +154,7 @@ static inline struct netlbl_af6list 
*__af6list_valid_rcu(struct list_head *s,
struct list_head *i = s;
struct netlbl_af6list *n = __af6list_entry(s);
while (i != h && !n->valid) {
-   i = rcu_dereference(i->next);
+   i = rcu_dereference(list_next_rcu(i));
n = __af6list_entry(i);
}
return n;
-- 
2.1.4

Re: [PATCH v2] net/sock: Update sk rcu iterator macro.

2017-10-23 Thread Tim Hansen 
Mon, Oct 23, 2017 at 06:42:50PM +, Levin, Alexander (Sasha Levin) wrote:
> On Mon, Oct 23, 2017 at 11:39:22AM -0400, Tim Hansen wrote:
> >Mark hlist nodes in sk rcu iterator as protected by the rcu.
> >hlist_next_rcu accomplishes this and silences the warnings
> >sparse throws for net/ipv4/udp.c and net/ipv6/udp.c.
> >
> >Found with make C=1 net/ipv4/udp.o on linux-next tag
> >next-20171009.
> >
> >Signed-off-by: Tim Hansen 
> >---
> > include/net/sock.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/net/sock.h b/include/net/sock.h
> >index 64e5ac41b9cf..96cb7b7e4924 100644
> >--- a/include/net/sock.h
> >+++ b/include/net/sock.h
> >@@ -737,10 +737,10 @@ static inline void sk_add_bind_node(struct sock *sk,
> >  *
> >  */
> > #define sk_for_each_entry_offset_rcu(tpos, pos, head, offset)   
> >\
> >-for (pos = rcu_dereference((head)->first); \
> >+for (pos = rcu_dereference(hlist_next_rcu((head)->first)); \
> 
> See below.
> 
> >  pos != NULL &&\
> > ({ tpos = (typeof(*tpos) *)((void *)pos - offset); 1;});   \
> >- pos = rcu_dereference(pos->next))
> >+ pos = rcu_dereference(hlist_next_rcu(pos->next)))
> 
> This will return the next-next node instead of just the next one. You
> probably want just hlist_next_rcu(pos) here.
> 
> -- 
> 
> Thanks,
> Sasha

Thanks Sasha, dumb oversight on my part. Sent in v3 of this patch.

[PATCH v3] net/sock: Update sk rcu iterator macro.

2017-10-23 Thread Tim Hansen 
Mark hlist node in sk rcu iterator as protected by the rcu.
hlist_next_rcu accomplishes this and silences the warnings
sparse throws.

Found with make C=1 net/ipv4/udp.o on linux-next tag 
next-20171009.

Signed-off-by: Tim Hansen 
---
 include/net/sock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 64e5ac41b9cf..14c73e8187a6 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -737,10 +737,10 @@ static inline void sk_add_bind_node(struct sock *sk,
  *
  */
 #define sk_for_each_entry_offset_rcu(tpos, pos, head, offset) \
-   for (pos = rcu_dereference((head)->first); \
+   for (pos = rcu_dereference(hlist_first_rcu(head)); \
 pos != NULL &&\
({ tpos = (typeof(*tpos) *)((void *)pos - offset); 1;});   \
-pos = rcu_dereference(pos->next))
+pos = rcu_dereference(hlist_next_rcu(pos)))
 
 static inline struct user_namespace *sk_user_ns(struct sock *sk)
 {
-- 
2.15.0.rc0

[PATCH v2] net/sock: Update sk rcu iterator macro.

2017-10-23 Thread Tim Hansen 
Mark hlist nodes in sk rcu iterator as protected by the rcu.
hlist_next_rcu accomplishes this and silences the warnings
sparse throws for net/ipv4/udp.c and net/ipv6/udp.c.

Found with make C=1 net/ipv4/udp.o on linux-next tag 
next-20171009.

Signed-off-by: Tim Hansen 
---
 include/net/sock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 64e5ac41b9cf..96cb7b7e4924 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -737,10 +737,10 @@ static inline void sk_add_bind_node(struct sock *sk,
  *
  */
 #define sk_for_each_entry_offset_rcu(tpos, pos, head, offset) \
-   for (pos = rcu_dereference((head)->first); \
+   for (pos = rcu_dereference(hlist_next_rcu((head)->first)); \
 pos != NULL &&\
({ tpos = (typeof(*tpos) *)((void *)pos - offset); 1;});   \
-pos = rcu_dereference(pos->next))
+pos = rcu_dereference(hlist_next_rcu(pos->next)))
 
 static inline struct user_namespace *sk_user_ns(struct sock *sk)
 {
-- 
2.15.0.rc0

[PATCH v2] net/core: Fix BUG to BUG_ON conditionals.

2017-10-09 Thread Tim Hansen 
Fix BUG() calls to use BUG_ON(conditional) macros.

This was found using make coccicheck M=net/core on linux next
tag next-2017092

Signed-off-by: Tim Hansen 
---
 net/core/skbuff.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d98c2e3ce2bf..34ce4c1a0f3c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1350,8 +1350,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t 
gfp_mask)
/* Set the tail pointer and length */
skb_put(n, skb->len);
 
-   if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
 
copy_skb_header(n, skb);
return n;
@@ -1449,8 +1448,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int 
ntail,
 
BUG_ON(nhead < 0);
 
-   if (skb_shared(skb))
-   BUG();
+   BUG_ON(skb_shared(skb));
 
size = SKB_DATA_ALIGN(size);
 
@@ -1595,9 +1593,8 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
head_copy_off = newheadroom - head_copy_len;
 
/* Copy the linear header and data. */
-   if (skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
- skb->len + head_copy_len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
+skb->len + head_copy_len));
 
copy_skb_header(n, skb);
 
@@ -1878,8 +1875,8 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
return NULL;
}
 
-   if (skb_copy_bits(skb, skb_headlen(skb), skb_tail_pointer(skb), delta))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, skb_headlen(skb),
+skb_tail_pointer(skb), delta));
 
/* Optimization: no fragments, no reasons to preestimate
 * size of pulled pages. Superb.
-- 
2.14.2

Re: [PATCH v2] net/core: Fix BUG to BUG_ON conditionals.

2017-10-08 Thread Tim Hansen 
Mistakenly sent the patch previously with a missing semicolon.
Apologies.

Fix BUG() calls to use BUG_ON(conditional) macros.

This was found using make coccicheck M=net/core on linux next
tag next-20170929

Signed-off-by: Tim Hansen 
---
 net/core/skbuff.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d98c2e3ce2bf..34ce4c1a0f3c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1350,8 +1350,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t 
gfp_mask)
/* Set the tail pointer and length */
skb_put(n, skb->len);
 
-   if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
 
copy_skb_header(n, skb);
return n;
@@ -1449,8 +1448,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int 
ntail,
 
BUG_ON(nhead < 0);
 
-   if (skb_shared(skb))
-   BUG();
+   BUG_ON(skb_shared(skb));
 
size = SKB_DATA_ALIGN(size);
 
@@ -1595,9 +1593,8 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
head_copy_off = newheadroom - head_copy_len;
 
/* Copy the linear header and data. */
-   if (skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
- skb->len + head_copy_len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
+skb->len + head_copy_len));
 
copy_skb_header(n, skb);
 
@@ -1878,8 +1875,8 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
return NULL;
}
 
-   if (skb_copy_bits(skb, skb_headlen(skb), skb_tail_pointer(skb), delta))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, skb_headlen(skb),
+skb_tail_pointer(skb), delta));
 
/* Optimization: no fragments, no reasons to preestimate
 * size of pulled pages. Superb.
-- 
2.14.2

[PATCH] net/core: Fix BUG to BUG_ON conditionals.

2017-10-08 Thread Tim Hansen 
Fix BUG() calls to use BUG_ON(conditional) macros.

This was found using make coccicheck M=net/core on linux next
tag next-20170929.

Signed-off-by: Tim Hansen 
---
 net/core/skbuff.c | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d98c2e3ce2bf..461516f45b33 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1350,8 +1350,7 @@ struct sk_buff *skb_copy(const struct sk_buff *skb, gfp_t 
gfp_mask)
/* Set the tail pointer and length */
skb_put(n, skb->len);
 
-   if (skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -headerlen, n->head, headerlen + skb->len));
 
copy_skb_header(n, skb);
return n;
@@ -1449,8 +1448,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int 
ntail,
 
BUG_ON(nhead < 0);
 
-   if (skb_shared(skb))
-   BUG();
+   BUG_ON(skb_shared(skb));
 
size = SKB_DATA_ALIGN(size);
 
@@ -1595,9 +1593,8 @@ struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
head_copy_off = newheadroom - head_copy_len;
 
/* Copy the linear header and data. */
-   if (skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
- skb->len + head_copy_len))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, -head_copy_len, n->head + head_copy_off,
+skb->len + head_copy_len));
 
copy_skb_header(n, skb);
 
@@ -1878,8 +1875,8 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
return NULL;
}
 
-   if (skb_copy_bits(skb, skb_headlen(skb), skb_tail_pointer(skb), delta))
-   BUG();
+   BUG_ON(skb_copy_bits(skb, skb_headlen(skb),
+skb_tail_pointer(skb), delta))
 
/* Optimization: no fragments, no reasons to preestimate
 * size of pulled pages. Superb.
-- 
2.14.2

[PATCH] net/ipv6: remove unused err variable on icmpv6_push_pending_frames

2017-10-05 Thread Tim Hansen 
int err is unused by icmpv6_push_pending_frames(), this patch returns removes 
the variable and returns the function with 0.

git bisect shows this variable has been around since linux has been in git in 
commit 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2.  

This was found by running make coccicheck M=net/ipv6/ on linus' tree on commit 
77ede3a014a32746002f7889211f0cecf4803163 (current HEAD as of this patch).

Signed-off-by: Tim Hansen 
---
 net/ipv6/icmp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 5acb544..aeb49b4 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -255,7 +255,6 @@ int icmpv6_push_pending_frames(struct sock *sk, struct 
flowi6 *fl6,
 {
struct sk_buff *skb;
struct icmp6hdr *icmp6h;
-   int err = 0;
 
skb = skb_peek(&sk->sk_write_queue);
if (!skb)
@@ -288,7 +287,7 @@ int icmpv6_push_pending_frames(struct sock *sk, struct 
flowi6 *fl6,
}
ip6_push_pending_frames(sk);
 out:
-   return err;
+   return 0;
 }
 
 struct icmpv6_msg {
-- 
2.1.4

[PATCH] net/ipv4: Remove unused variable in route.c

2017-10-04 Thread Tim Hansen 
int rc is unmodified after initalization in net/ipv4/route.c, this patch simply 
cleans up that variable and returns 0.

This was found with coccicheck M=net/ipv4/ on linus' tree.

Signed-off-by: Tim Hansen 
---
 net/ipv4/route.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 94d4cd2..02b6be9 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3032,7 +3032,6 @@ struct ip_rt_acct __percpu *ip_rt_acct __read_mostly;
 
 int __init ip_rt_init(void)
 {
-   int rc = 0;
int cpu;
 
ip_idents = kmalloc(IP_IDENTS_SZ * sizeof(*ip_idents), GFP_KERNEL);
@@ -3089,7 +3088,7 @@ int __init ip_rt_init(void)
 #endif
register_pernet_subsys(&rt_genid_ops);
register_pernet_subsys(&ipv4_inetpeer_ops);
-   return rc;
+   return 0;
 }
 
 #ifdef CONFIG_SYSCTL
-- 
2.1.4

Re: [PATCH] net/ipv4: Update sk_for_each_entry_offset_rcu macro to utilize rcu methods hlist_next_rcu. This fixes the warnings thrown by sparse regarding net/ipv4/udp.c on line 1974.

2017-09-27 Thread Tim Hansen 
> Tue, Sep 26, 2017 at 08:03:39PM -0700, David Miller wrote:
> From: Tim Hansen 
> Date: Tue, 26 Sep 2017 20:54:05 -0400
>
> > Signed-off-by: Tim Hansen 
>
> This is a poor patch submission on many levels.
>

Apologies Dave, this is my first patch. I appreciate the quick review and 
helpful feedback.

> But the main problem, is that there is no use of
> sk_for_each_entry_offset_rcu() in any of my networking kernel trees.
>

Using the get_maintainers.pl on include/net/sock.h brings up your name and the 
netdev mailing list.  Forgive me if I'm misunderstanding what you mean by this?

> Referencing code by line number never works, you have to mention
> what version of the kernel, what tree, and where in what fucntion
> the problem is occurring.
>

I am using your tree net tree for now: 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git on HEAD. HEAD is 
c2cc187e53011c1c4931055984657da9085c763b for me currently on your tree.

Before I was on the 4.13 tag pulled from linus' tree.

The line number is indeed useless in hindsight since there are many different 
trees. I won't do that again.

Using sparse 0.5.0 on HEAD of your net tree, I run make C=1 net/ipv4/. It 
throws the error:

"net/ipv4/udp.c:1981:9: error: incompatible types in comparison expression 
(different address spaces)
net/ipv4/udp.c:1981:9: error: incompatible types in comparison expression 
(different address spaces)"

This points to the function sk_for_each_entry_offset_rcu() in 
__udp4_lib_mcast_deliver in net/ipv4/udp.c.

Inspecting this macro in include/net/sock.h is what lead to this patch.

Applying the patch silences those warnings but clearly this is -not- a proper 
way of fixing the error. Any guidance would be greatly appreciated.

> Secondly, sk_for_each_entry_offset_rcu() is not meant to be used
> in _raw() contexts.  This is why it's not called
> sk_for_each_entry_offset_rcu_raw().

Absolutely makes sense. I am not familar with the kernel naming standards fully 
yet but this is obvious in hindsight.

>
> The sparse warning is probably legitimate, and points to a bug.
>
> But nobody can tell where becuase you haven't told us what tree
> and where this happens.

Hopefully my reply has enough detail for reproduction and further debugging. 
Please let me know if I should supply any additional information.

[PATCH] net/ipv4: Update sk_for_each_entry_offset_rcu macro to utilize rcu methods hlist_next_rcu. This fixes the warnings thrown by sparse regarding net/ipv4/udp.c on line 1974.

2017-09-26 Thread Tim Hansen 
Signed-off-by: Tim Hansen 
---
 include/net/sock.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index aeeec62992ca..516289f6404b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -732,10 +732,10 @@ static inline void sk_add_bind_node(struct sock *sk,
  *
  */
 #define sk_for_each_entry_offset_rcu(tpos, pos, head, offset) \
-   for (pos = rcu_dereference((head)->first); \
+   for (pos = rcu_dereference_raw(hlist_next_rcu((head)->first)); \
 pos != NULL &&\
({ tpos = (typeof(*tpos) *)((void *)pos - offset); 1;});   \
-pos = rcu_dereference(pos->next))
+pos = rcu_dereference_raw(hlist_next_rcu(pos->next)))
 
 static inline struct user_namespace *sk_user_ns(struct sock *sk)
 {
-- 
2.14.1