On Wed, 2014-05-14 at 14:40 +0100, Zoltan Kiss wrote:
> Hi,
>
> Recently I've investigated issues around SKBTX_DEV_ZEROCOPY skbs where
> the frags list were modified. I came across this function skb_shift(),
> which moves frags between skbs. And there are a lot more of such kind,
> skb_split or skb_try_coalesce, for example.
> It could be a dangerous thing if a frag is referenced from an skb which
> doesn't have the original destructor_arg, and to avoid that
> skb_orphan_frags should be called. Although probably these functions are
> not normally touched in usual usecases, I think it would be useful to
> review core skb functions proactively and add an skb_orphan_frags
> everywhere where the frags could be referenced from other places.
> Any opinion about this?
For skb_shift(), it is currently used from tcp stack only, where
this SKBTX_DEV_ZEROCOPY thing is not used, so I do not think there is a
bug for the moment.
I already gave a patch for skb_try_coalesce() : For this one we do not
wan skb_orphan_frags() overhead. Its simply better in this case to
abort.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 1b62343f5837..85995a14aafc 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3838,7 +3839,10 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
return true;
}
- if (skb_has_frag_list(to) || skb_has_frag_list(from))
+ if (skb_has_frag_list(to) ||
+ skb_has_frag_list(from) ||
+ (skb_shinfo(to)->tx_flags & SKBTX_DEV_ZEROCOPY) ||
+ (skb_shinfo(from)->tx_flags & SKBTX_DEV_ZEROCOPY))
return false;
if (skb_headlen(from) != 0) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
