Re: [PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
On Thu, May 27, 2010 at 01:02:09PM +0800, Ang Way Chuang wrote:
ULE (Unidirectional Lightweight Encapsulation RFC 4326)
decapsulation code has a bug that incorrectly treats ULE SNDU packed
into the remaining 2 or 3 bytes of a MPEG2-TS frame as having
invalid pointer field on the subsequent MPEG2-TS frame.
This patch was generated and tested against the latest Linus's pre
2.6.35-rc1 tree.
Signed-off-by: Ang Way Chuang wc...@nav6.org
Looks good to me, thanks for the updated version. Good catch noting that
error needed to be reset to false after it was handled, I'd missed that.
Acked-by: Jarod Wilson ja...@redhat.com
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index f6dac2b..6c3a8a0 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -351,6 +351,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
const u8 *ts, *ts_end, *from_where = NULL;
u8 ts_remain = 0, how_much = 0, new_ts = 1;
struct ethhdr *ethh = NULL;
+ bool error = false;
#ifdef ULE_DEBUG
/* The code inside ULE_DEBUG keeps a history of the last 100 TS cells
processed. */
@@ -460,10 +461,16 @@ static void dvb_net_ule( struct net_device *dev, const
u8 *buf, size_t buf_len )
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ error = true;
+
dev_kfree_skb(priv-ule_skb);
+ }
+
+ if (error ||
priv-ule_sndu_remain) {
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
+ error = false;
}
+
reset_ule(priv);
priv-need_pusi = 1;
continue;
@@ -535,6 +542,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -544,6 +552,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -557,6 +566,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
Jarod Wilson
ja...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against the latest Linus's pre
2.6.35-rc1 tree.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index f6dac2b..6c3a8a0 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -351,6 +351,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
const u8 *ts, *ts_end, *from_where = NULL;
u8 ts_remain = 0, how_much = 0, new_ts = 1;
struct ethhdr *ethh = NULL;
+ bool error = false;
#ifdef ULE_DEBUG
/* The code inside ULE_DEBUG keeps a history of the last 100 TS cells
processed. */
@@ -460,10 +461,16 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ error = true;
+
dev_kfree_skb(priv-ule_skb);
+ }
+
+ if (error ||
priv-ule_sndu_remain) {
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
+ error = false;
}
+
reset_ule(priv);
priv-need_pusi = 1;
continue;
@@ -535,6 +542,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -544,6 +552,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -557,6 +566,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
On Fri, May 21, 2010 at 11:40:34AM +0800, Ang Way Chuang wrote: Hi Jarod, Thanks for the review. My answers are inlined. Jarod Wilson wrote: On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote: ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation code has a bug that incorrectly treats ULE SNDU packed into the remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer field on the subsequent MPEG2-TS frame. This patch was generated and tested against v2.6.34-rc6. I suspect that this bug was introduced in kernel version 2.6.15, but had not verified it. Care has been taken not to introduce more bug by fixing this bug, but please scrutinize the code because I always produces buggy code. ... @@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len ) from_where += 2; } + priv-ule_sndu_remain = priv-ule_sndu_len + 2; /* * State of current TS: * ts_remain (remaining bytes in the current TS cell) Is this *always* true? Your description says ...the remaining 2 or 3 bytes, indicating this could sometimes need to be +3. Is +0 also a possibility? Not sure whether I understand your question correctly. Here is my attempt to answer your question. The encapsulation format always mandate that at least of 2 bytes of ULE SNDU (the LENGTH field) must be present within a MPEG2-TS frame. So, if only 1 byte of the ULE SNDU get packed into the remaining MPEG2-TS frame, then it is invalid. Of course, there is no issue regarding 0 byte as that would be the case of filling MPEG2-TS frame up to its boundary. New ULE SNDU will have to packed into the next MPEG2-TS frame in that case. Now the problem with existing code is the interpretation of remainder length when 2 or 3 bytes of ULE SNDU are packed into the remainder of MPEG2-TS frame. In the 2 bytes case, only the LENGTH field is available while in the case 3 bytes, only the 1st octet of the 2-octets TYPE field and the LENGTH field are available. The ule_sndu_remain should carry the value of length of ULE SNDU following the the TYPE field. Technically, this would mean that remainder byte of ULE SNDU that need to be received is going to be: Value(LENGTH) + 2 (We owe 2 bytes of TYPE field here) if only 2 bytes of ULE SNDU is packed (as in the case of case 0: at line 550). This is addressed by adding the priv-ule_sndu_remain = priv-ule_sndu_len + 2; Value(LENGTH) + 1 (We owe 1 byte of TYPE field here) if 3 bytes of ULE SNDU is packed (as in the case of case 1: at 545). This is addressed by adding priv-ule_sndu_remain--; If complete ULE header (= 4 bytes) is available: priv-ule_sndu_remain = priv-ule_sndu_len; at line 582 takes care of the rest and it works just fine in the existing code. Due to the wrong interpretation of remaining length of ULE SNDU when 2 or 3 bytes of ULE SNDU are packed into a MPEG2-TS frame, the subsequent checking of payload pointer (line 455) always fails leading to unnecessary packet drops. Looking back at the fix after a few months, I had trouble understanding how these few lines fixed the problem at first glance. Yeah, my question was whether or not the +2 would account for both the +2 bytes and +3 bytes situations, and it seems that's handled appropriately by the ts_remain switch. Thank you for the detailed explanation. If you'd alter that nested check for freeing the skb and give it a quick test, I'm happy to throw an acked-by or reviewed-by on a followup submission. -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
Jarod Wilson wrote: On Fri, May 21, 2010 at 11:40:34AM +0800, Ang Way Chuang wrote: Hi Jarod, Thanks for the review. My answers are inlined. Jarod Wilson wrote: On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote: ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation code has a bug that incorrectly treats ULE SNDU packed into the remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer field on the subsequent MPEG2-TS frame. This patch was generated and tested against v2.6.34-rc6. I suspect that this bug was introduced in kernel version 2.6.15, but had not verified it. Care has been taken not to introduce more bug by fixing this bug, but please scrutinize the code because I always produces buggy code. ... @@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len ) from_where += 2; } + priv-ule_sndu_remain = priv-ule_sndu_len + 2; /* * State of current TS: * ts_remain (remaining bytes in the current TS cell) Is this *always* true? Your description says ...the remaining 2 or 3 bytes, indicating this could sometimes need to be +3. Is +0 also a possibility? Not sure whether I understand your question correctly. Here is my attempt to answer your question. The encapsulation format always mandate that at least of 2 bytes of ULE SNDU (the LENGTH field) must be present within a MPEG2-TS frame. So, if only 1 byte of the ULE SNDU get packed into the remaining MPEG2-TS frame, then it is invalid. Of course, there is no issue regarding 0 byte as that would be the case of filling MPEG2-TS frame up to its boundary. New ULE SNDU will have to packed into the next MPEG2-TS frame in that case. Now the problem with existing code is the interpretation of remainder length when 2 or 3 bytes of ULE SNDU are packed into the remainder of MPEG2-TS frame. In the 2 bytes case, only the LENGTH field is available while in the case 3 bytes, only the 1st octet of the 2-octets TYPE field and the LENGTH field are available. The ule_sndu_remain should carry the value of length of ULE SNDU following the the TYPE field. Technically, this would mean that remainder byte of ULE SNDU that need to be received is going to be: Value(LENGTH) + 2 (We owe 2 bytes of TYPE field here) if only 2 bytes of ULE SNDU is packed (as in the case of case 0: at line 550). This is addressed by adding the priv-ule_sndu_remain = priv-ule_sndu_len + 2; Value(LENGTH) + 1 (We owe 1 byte of TYPE field here) if 3 bytes of ULE SNDU is packed (as in the case of case 1: at 545). This is addressed by adding priv-ule_sndu_remain--; If complete ULE header (= 4 bytes) is available: priv-ule_sndu_remain = priv-ule_sndu_len; at line 582 takes care of the rest and it works just fine in the existing code. Due to the wrong interpretation of remaining length of ULE SNDU when 2 or 3 bytes of ULE SNDU are packed into a MPEG2-TS frame, the subsequent checking of payload pointer (line 455) always fails leading to unnecessary packet drops. Looking back at the fix after a few months, I had trouble understanding how these few lines fixed the problem at first glance. Yeah, my question was whether or not the +2 would account for both the +2 bytes and +3 bytes situations, and it seems that's handled appropriately by the ts_remain switch. Thank you for the detailed explanation. If you'd alter that nested check for freeing the skb and give it a quick test, I'm happy to throw an acked-by or reviewed-by on a followup submission. Got it. Thank you. I shall get that patch to you next week because I'm not in the lab now. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote:
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.34-rc6. I suspect
that this bug was introduced in kernel version 2.6.15, but had not
verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code because I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 441c064..35a4afb 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+ dev_kfree_skb(
priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
That code block looks odd that way, but after staring at it for a minute,
it makes sense. Another way to do it that might read cleaner (and reduce
excessive tab indent levels) would be to add a 'bool errors', then:
bool errors = false;
...
if (priv-ule_skb) {
errors = true;
dev_kfree_skb(priv-ule_skb);
}
if (errors || priv-ule_sndu_remain) {
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
Is this *always* true? Your description says ...the remaining 2 or 3
bytes, indicating this could sometimes need to be +3. Is +0 also a
possibility?
@@ -543,6 +545,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -556,6 +559,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
Jarod Wilson
ja...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
Hi Jarod,
Thanks for the review. My answers are inlined.
Jarod Wilson wrote:
On Thu, May 06, 2010 at 02:52:22PM -, Ang Way Chuang wrote:
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.34-rc6. I suspect
that this bug was introduced in kernel version 2.6.15, but had not
verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code because I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 441c064..35a4afb 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+ dev_kfree_skb(
priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
That code block looks odd that way, but after staring at it for a minute,
it makes sense. Another way to do it that might read cleaner (and reduce
excessive tab indent levels) would be to add a 'bool errors', then:
bool errors = false;
...
if (priv-ule_skb) {
errors = true;
dev_kfree_skb(priv-ule_skb);
}
if (errors || priv-ule_sndu_remain) {
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
Yeap, I'm a lazy person.
@@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
Is this *always* true? Your description says ...the remaining 2 or 3
bytes, indicating this could sometimes need to be +3. Is +0 also a
possibility?
Not sure whether I understand your question correctly. Here is my attempt to answer your question.
The encapsulation format always mandate that at least of 2 bytes of ULE SNDU (the LENGTH field) must
be present within a MPEG2-TS frame. So, if only 1 byte of the ULE SNDU get packed into the
remaining MPEG2-TS frame, then it is invalid. Of course, there is no issue regarding 0 byte as that
would be the case of filling MPEG2-TS frame up to its boundary. New ULE SNDU will have to packed
into the next MPEG2-TS frame in that case.
Now the problem with existing code is the interpretation of remainder length when 2 or 3 bytes of ULE
SNDU are packed into the remainder of MPEG2-TS frame. In the 2 bytes case, only the LENGTH field is
available while in the case 3 bytes, only the 1st octet of the 2-octets TYPE field and the LENGTH field
are available. The ule_sndu_remain should carry the value of length of ULE SNDU following the the TYPE
field. Technically, this would mean that remainder byte of ULE SNDU that need to be received is going
to be:
Value(LENGTH) + 2 (We owe 2 bytes of TYPE field here) if only 2 bytes of ULE SNDU is packed (as in the
case of case 0: at line 550).
This is addressed by adding the priv-ule_sndu_remain = priv-ule_sndu_len + 2;
Value(LENGTH) + 1 (We owe 1 byte of TYPE field here) if 3 bytes of ULE SNDU is packed (as in the case of
case 1: at 545).
This is addressed by adding priv-ule_sndu_remain--;
If complete ULE header (= 4 bytes) is available:
priv-ule_sndu_remain = priv-ule_sndu_len; at line 582 takes care of the rest and it works just fine in
the existing code.
Due to the wrong interpretation of remaining length of ULE SNDU when 2 or 3 bytes of ULE SNDU are packed
into a MPEG2-TS frame, the subsequent checking of payload pointer (line 455) always fails leading to
unnecessary packet drops.
Looking back at the fix after a few months, I had trouble understanding how these few lines fixed the
problem at first glance.
@@ -543,6
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.34-rc6. I suspect
that this bug was introduced in kernel version 2.6.15, but had not
verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code because I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 441c064..35a4afb 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+ dev_kfree_skb(
priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -534,6 +535,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -543,6 +545,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -556,6 +559,7 @@ static void dvb_net_ule( struct net_device *dev, const u8
*buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
Okay, resending. Hope it won't do line wrapping.
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.32-rc8. Similar patch
was applied and tested using 2.6.27 which is similar to the latest
dvb_net.c, except for network device statistical data structure. I
suspect that this bug was introduced in kernel version 2.6.15, but had
not verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code for I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 0241a7c..7e0db86 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+ dev_kfree_skb(
priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -533,6 +534,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -542,6 +544,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -555,6 +558,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
Sorry, line wrap again. I shall test and fix the problem first before
resending the patch.
On Tue, Nov 24, 2009 at 4:04 PM, Ang Way Chuang wcan...@gmail.com wrote:
Okay, resending. Hope it won't do line wrapping.
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.32-rc8. Similar patch
was applied and tested using 2.6.27 which is similar to the latest
dvb_net.c, except for network device statistical data structure. I
suspect that this bug was introduced in kernel version 2.6.15, but had
not verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code for I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 0241a7c..7e0db86 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+
dev_kfree_skb( priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -533,6 +534,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -542,6 +544,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -555,6 +558,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
On Mon, Nov 23, 2009 at 05:37:57PM +0800, Ang Way Chuang wrote: --- a/drivers/media/dvb/dvb-core/dvb_net.c +++ b/drivers/media/dvb/dvb-core/dvb_net.c @@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev, const u8 *buf, size_t buf_len ) Your email client line broke the line starting with @@ into 2 lines so the patch doesn't apply. Could you resend the patch without line wrapping? regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.32-rc8. Similar patch
was applied and tested using 2.6.27 which is similar to the latest
dvb_net.c, except for network device statistical data structure. I
suspect that this bug was introduced in kernel version 2.6.15, but had
not verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code for I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 0241a7c..7e0db86 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded SNDU,
reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+ dev_kfree_skb(
priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -533,6 +534,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -542,6 +544,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -555,6 +558,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the
remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer
field on the subsequent MPEG2-TS frame.
This patch was generated and tested against v2.6.32-rc8. Similar patch
was applied and tested using 2.6.27 which is similar to the latest
dvb_net.c, except for network device statistical data structure. I
suspect that this bug was introduced in kernel version 2.6.15, but had
not verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code for I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 0241a7c..7e0db86 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -458,8 +458,9 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
field: %u.\n,
priv-ts_count, *from_where);
/* Drop partly decoded
SNDU, reset state, resync on PUSI. */
- if (priv-ule_skb) {
- dev_kfree_skb(
priv-ule_skb );
+ if (priv-ule_skb ||
priv-ule_sndu_remain) {
+ if (priv-ule_skb)
+
dev_kfree_skb( priv-ule_skb );
dev-stats.rx_errors++;
dev-stats.rx_frame_errors++;
}
@@ -533,6 +534,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -542,6 +544,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0] 8;
priv-ule_sndu_type_1 = 1; /*
first byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -555,6 +558,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
default: /* complete ULE header is
present in current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /*
points to payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation code has a bug that incorrectly treats ULE SNDU packed into the remaining 2 or 3 bytes of a MPEG2-TS frame as having invalid pointer field on the subsequent MPEG2-TS frame. This patch was generated against v2.6.32-rc7, however it wasn't tested using that kernel. Similar patch was applied and tested using 2.6.27 which is similar to the latest dvb_net.c, except for network device statistical data structure. I suspect that this bug was introduced in kernel version 2.6.15, but had not verified it. Care has been taken not to introduce more bug by fixing this bug, but please scrutinize the code for I always produces buggy code. Signed-off-by: Ang Way Chuang wc...@nav6.org -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb-core: Fix ULE decapsulation bug when less than 4 bytes of ULE SNDU is packed into the remaining bytes of a MPEG2-TS frame
ULE (Unidirectional Lightweight Encapsulation RFC 4326) decapsulation
code has a bug that incorrectly treats ULE SNDU packed into the remaining
2 or 3 bytes of a MPEG2-TS frame as having invalid pointer field on the
subsequent MPEG2-TS frame.
This patch was generated against v2.6.32-rc7, however it wasn't tested
using that kernel. Similar patch was applied and tested using 2.6.27 which
is similar to the latest dvb_net.c, except for network device statistical data
structure. I suspect that this bug was introduced in kernel version 2.6.15,
but had not verified it.
Care has been taken not to introduce more bug by fixing this bug, but
please scrutinize the code for I always produces buggy code.
Signed-off-by: Ang Way Chuang wc...@nav6.org
---
diff --git a/drivers/media/dvb/dvb-core/dvb_net.c
b/drivers/media/dvb/dvb-core/dvb_net.c
index 0241a7c..a521395 100644
--- a/drivers/media/dvb/dvb-core/dvb_net.c
+++ b/drivers/media/dvb/dvb-core/dvb_net.c
@@ -533,6 +533,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
from_where += 2;
}
+ priv-ule_sndu_remain = priv-ule_sndu_len + 2;
/*
* State of current TS:
* ts_remain (remaining bytes in the current TS cell)
@@ -542,6 +543,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
*/
switch (ts_remain) {
case 1:
+ priv-ule_sndu_remain--;
priv-ule_sndu_type = from_where[0]
8;
priv-ule_sndu_type_1 = 1; /* first
byte of ule_type is set. */
ts_remain -= 1; from_where += 1;
@@ -555,6 +557,7 @@ static void dvb_net_ule( struct net_device *dev,
const u8 *buf, size_t buf_len )
default: /* complete ULE header is present in
current TS. */
/* Extract ULE type field. */
if (priv-ule_sndu_type_1) {
+ priv-ule_sndu_type_1 = 0;
priv-ule_sndu_type |=
from_where[0];
from_where += 1; /* points to
payload start. */
ts_remain -= 1;
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
