Skip to content

Commit

Permalink
[NET]: Revert sk_buff walker cleanups.
Browse files Browse the repository at this point in the history
This reverts eefa390

The simplification made in that change works with the assumption that
the 'offset' parameter to these functions is always positive or zero,
which is not true.  It can be and often is negative in order to access
SKB header values in front of skb->data.

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Apr 27, 2007
1 parent 50f732e commit 1a028e5
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 86 deletions.
25 changes: 16 additions & 9 deletions net/appletalk/ddp.c
Original file line number Diff line number Diff line change
Expand Up @@ -937,11 +937,11 @@ static unsigned long atalk_sum_partial(const unsigned char *data,
static unsigned long atalk_sum_skb(const struct sk_buff *skb, int offset,
int len, unsigned long sum)
{
int end = skb_headlen(skb);
int start = skb_headlen(skb);
int i, copy;

/* checksum stuff in header space */
if ((copy = end - offset) > 0) {
if ( (copy = start - offset) > 0) {
if (copy > len)
copy = len;
sum = atalk_sum_partial(skb->data + offset, copy, sum);
Expand All @@ -953,41 +953,48 @@ static unsigned long atalk_sum_skb(const struct sk_buff *skb, int offset,

/* checksum stuff in frags */
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
BUG_TRAP(len >= 0);
int end;

end = offset + skb_shinfo(skb)->frags[i].size;
BUG_TRAP(start <= offset + len);

end = start + skb_shinfo(skb)->frags[i].size;
if ((copy = end - offset) > 0) {
u8 *vaddr;
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];

if (copy > len)
copy = len;
vaddr = kmap_skb_frag(frag);
sum = atalk_sum_partial(vaddr + frag->page_offset,
copy, sum);
sum = atalk_sum_partial(vaddr + frag->page_offset +
offset - start, copy, sum);
kunmap_skb_frag(vaddr);

if (!(len -= copy))
return sum;
offset += copy;
}
start = end;
}

if (skb_shinfo(skb)->frag_list) {
struct sk_buff *list = skb_shinfo(skb)->frag_list;

for (; list; list = list->next) {
BUG_TRAP(len >= 0);
int end;

BUG_TRAP(start <= offset + len);

end = offset + list->len;
end = start + list->len;
if ((copy = end - offset) > 0) {
if (copy > len)
copy = len;
sum = atalk_sum_skb(list, 0, copy, sum);
sum = atalk_sum_skb(list, offset - start,
copy, sum);
if ((len -= copy) == 0)
return sum;
offset += copy;
}
start = end;
}
}

Expand Down
50 changes: 33 additions & 17 deletions net/core/datagram.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ EXPORT_SYMBOL(skb_kill_datagram);
int skb_copy_datagram_iovec(const struct sk_buff *skb, int offset,
struct iovec *to, int len)
{
int end = skb_headlen(skb);
int i, copy = end - offset;
int start = skb_headlen(skb);
int i, copy = start - offset;

/* Copy header. */
if (copy > 0) {
Expand All @@ -263,9 +263,11 @@ int skb_copy_datagram_iovec(const struct sk_buff *skb, int offset,

/* Copy paged appendix. Hmm... why does this look so complicated? */
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
BUG_TRAP(len >= 0);
int end;

end = offset + skb_shinfo(skb)->frags[i].size;
BUG_TRAP(start <= offset + len);

end = start + skb_shinfo(skb)->frags[i].size;
if ((copy = end - offset) > 0) {
int err;
u8 *vaddr;
Expand All @@ -275,33 +277,39 @@ int skb_copy_datagram_iovec(const struct sk_buff *skb, int offset,
if (copy > len)
copy = len;
vaddr = kmap(page);
err = memcpy_toiovec(to, vaddr + frag->page_offset,
copy);
err = memcpy_toiovec(to, vaddr + frag->page_offset +
offset - start, copy);
kunmap(page);
if (err)
goto fault;
if (!(len -= copy))
return 0;
offset += copy;
}
start = end;
}

if (skb_shinfo(skb)->frag_list) {
struct sk_buff *list = skb_shinfo(skb)->frag_list;

for (; list; list = list->next) {
BUG_TRAP(len >= 0);
int end;

BUG_TRAP(start <= offset + len);

end = offset + list->len;
end = start + list->len;
if ((copy = end - offset) > 0) {
if (copy > len)
copy = len;
if (skb_copy_datagram_iovec(list, 0, to, copy))
if (skb_copy_datagram_iovec(list,
offset - start,
to, copy))
goto fault;
if ((len -= copy) == 0)
return 0;
offset += copy;
}
start = end;
}
}
if (!len)
Expand All @@ -315,9 +323,9 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
u8 __user *to, int len,
__wsum *csump)
{
int end = skb_headlen(skb);
int start = skb_headlen(skb);
int pos = 0;
int i, copy = end - offset;
int i, copy = start - offset;

/* Copy header. */
if (copy > 0) {
Expand All @@ -336,9 +344,11 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
}

for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
BUG_TRAP(len >= 0);
int end;

end = offset + skb_shinfo(skb)->frags[i].size;
BUG_TRAP(start <= offset + len);

end = start + skb_shinfo(skb)->frags[i].size;
if ((copy = end - offset) > 0) {
__wsum csum2;
int err = 0;
Expand All @@ -350,7 +360,8 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
copy = len;
vaddr = kmap(page);
csum2 = csum_and_copy_to_user(vaddr +
frag->page_offset,
frag->page_offset +
offset - start,
to, copy, 0, &err);
kunmap(page);
if (err)
Expand All @@ -362,20 +373,24 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
to += copy;
pos += copy;
}
start = end;
}

if (skb_shinfo(skb)->frag_list) {
struct sk_buff *list = skb_shinfo(skb)->frag_list;

for (; list; list=list->next) {
BUG_TRAP(len >= 0);
int end;

BUG_TRAP(start <= offset + len);

end = offset + list->len;
end = start + list->len;
if ((copy = end - offset) > 0) {
__wsum csum2 = 0;
if (copy > len)
copy = len;
if (skb_copy_and_csum_datagram(list, 0,
if (skb_copy_and_csum_datagram(list,
offset - start,
to, copy,
&csum2))
goto fault;
Expand All @@ -386,6 +401,7 @@ static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
to += copy;
pos += copy;
}
start = end;
}
}
if (!len)
Expand Down
Loading

0 comments on commit 1a028e5

Please sign in to comment.