[PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[YongSu Yoo]

Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>

The function dvb_ringbuffer_pkt_next in
/linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
which searches the idx of the next valid packet in the ring
buffer of the ca->slot_info[slot].rx_buffer at
/linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
has the following problem.
In calculating the amounts of the consumed address of the ring
buffer, if the read address(rbuf->pread) of the ring buffer is
smaller than the idx, the amounts of the searched address
should be (idx - rbuf->pread),
whereas if the read address(rbuf->pread) of the ring buffer is
larger than the idx, the amounts of the consumed address should
be (idx - rbuf->pread + rbug->size). But there exists an
incorrect logic that the rbug-size was not properly added on
(idx - rbug->pread) in the later case. With this commit, we
fixed this bug.
---
 drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c b/drivers/media/dvb-core/dvb_ringbuffer.c
index d1d471af0636..7d4558de8e83 100644
--- a/drivers/media/dvb-core/dvb_ringbuffer.c
+++ b/drivers/media/dvb-core/dvb_ringbuffer.c
@@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct dvb_ringbuffer *rbuf, size_t idx, size_t*
 		idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
 	}
 
-	consumed = (idx - rbuf->pread) % rbuf->size;
+	consumed = (idx - rbuf->pread);
+	if (consumed < 0)
+		consumed += rbuf->size;
 
 	while((dvb_ringbuffer_avail(rbuf) - consumed) > DVB_RINGBUFFER_PKTHDRSIZE) {
 
-- 
2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Hi ~

How is this patch going ?
Can you share current status ?

Thank you


2022년 6월 23일 (목) 오후 7:35, YongSu Yoo <yongsuyoo0215@gmail.com>님이 작성:
>
> Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
>
> The function dvb_ringbuffer_pkt_next in
> /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> which searches the idx of the next valid packet in the ring
> buffer of the ca->slot_info[slot].rx_buffer at
> /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> has the following problem.
> In calculating the amounts of the consumed address of the ring
> buffer, if the read address(rbuf->pread) of the ring buffer is
> smaller than the idx, the amounts of the searched address
> should be (idx - rbuf->pread),
> whereas if the read address(rbuf->pread) of the ring buffer is
> larger than the idx, the amounts of the consumed address should
> be (idx - rbuf->pread + rbug->size). But there exists an
> incorrect logic that the rbug-size was not properly added on
> (idx - rbug->pread) in the later case. With this commit, we
> fixed this bug.
> ---
>  drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c b/drivers/media/dvb-core/dvb_ringbuffer.c
> index d1d471af0636..7d4558de8e83 100644
> --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct dvb_ringbuffer *rbuf, size_t idx, size_t*
>                 idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
>         }
>
> -       consumed = (idx - rbuf->pread) % rbuf->size;
> +       consumed = (idx - rbuf->pread);
> +       if (consumed < 0)
> +               consumed += rbuf->size;
>
>         while((dvb_ringbuffer_avail(rbuf) - consumed) > DVB_RINGBUFFER_PKTHDRSIZE) {
>
> --
> 2.17.1
>

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Hans Petter Selasky]

On 6/26/22 23:11, 유용수 wrote:
> Hi ~
> 
> How is this patch going ?
> Can you share current status ?
> 
> Thank you
> 

Hi Yongsu,

Linux guys can sometimes take a long time to include patches speaking 
weeks and months. For now I've added your patch to multimedia/webcamd 
(v5.17.1.1) which runs under FreeBSD 13.1 (not Linux).

https://github.com/hselasky/webcamd/commit/0e4d4959a2aea2e6a88d316eb943592fe0b23d09

--HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear Hans Petter Selasky
Thank you for your response and good advice and informations

Dear All
How is this patch going ?
Is there anyone who can share the current status ?

2022년 6월 30일 (목) 오후 10:42, Hans Petter Selasky <hps@selasky.org>님이 작성:
>
> On 6/26/22 23:11, 유용수 wrote:
> > Hi ~
> >
> > How is this patch going ?
> > Can you share current status ?
> >
> > Thank you
> >
>
> Hi Yongsu,
>
> Linux guys can sometimes take a long time to include patches speaking
> weeks and months. For now I've added your patch to multimedia/webcamd
> (v5.17.1.1) which runs under FreeBSD 13.1 (not Linux).
>
> https://github.com/hselasky/webcamd/commit/0e4d4959a2aea2e6a88d316eb943592fe0b23d09
>
> --HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear All

2 months have already passed since I sent the first E-mail.
Can you pay your attention to this patch ?
Can you take care of this patch ?

2022년 7월 7일 (목) 오후 5:36, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>
> Dear Hans Petter Selasky
> Thank you for your response and good advice and informations
>
> Dear All
> How is this patch going ?
> Is there anyone who can share the current status ?
>
> 2022년 6월 30일 (목) 오후 10:42, Hans Petter Selasky <hps@selasky.org>님이 작성:
> >
> > On 6/26/22 23:11, 유용수 wrote:
> > > Hi ~
> > >
> > > How is this patch going ?
> > > Can you share current status ?
> > >
> > > Thank you
> > >
> >
> > Hi Yongsu,
> >
> > Linux guys can sometimes take a long time to include patches speaking
> > weeks and months. For now I've added your patch to multimedia/webcamd
> > (v5.17.1.1) which runs under FreeBSD 13.1 (not Linux).
> >
> > https://github.com/hselasky/webcamd/commit/0e4d4959a2aea2e6a88d316eb943592fe0b23d09
> >
> > --HPS

Fwd: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

Dear Alls

Can you check the Below E-mail ?
Can you share how is the below patch going ?

---------- Forwarded message ---------
보낸사람: 유용수 <yongsuyoo0215@gmail.com>
Date: 2022년 5월 22일 (일) 오후 3:36
Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
To: <mchehab@kernel.org>
Cc: <0215yys@hanmail.net>, <yongsu.yoo@lge.com>, 유용수 <yongsuyoo0215@gmail.com>


Dear Mauro Carvalho Chehab
My name is YongSu Yoo

I found an incorrect logic at the function of dvb_ringbuffer_pkt_next in
/linux-next/drviers/media/dvb-core/dvb_ringbuffer.c.
I correct the bug like the attached file (
0001-media-dvb_ringbuffer-Fix-a-bug-in-dvb_ringbuffer.c.patch ).
Please check the attached file.

Thank you

[-- Attachment #2: 0001-media-dvb_ringbuffer-Fix-a-bug-in-dvb_ringbuffer.c.patch --]
[-- Type: application/x-patch, Size: 1821 bytes --]

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear Alls

How is my kernel modification going ?
Can you check my kernel modification ?
and give me the feedback ?

2022년 6월 1일 (수) 오후 9:54, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>
> Dear Alls
>
> Can you check the Below E-mail ?
> Can you share how is the below patch going ?
>
> ---------- Forwarded message ---------
> 보낸사람: 유용수 <yongsuyoo0215@gmail.com>
> Date: 2022년 5월 22일 (일) 오후 3:36
> Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> To: <mchehab@kernel.org>
> Cc: <0215yys@hanmail.net>, <yongsu.yoo@lge.com>, 유용수 <yongsuyoo0215@gmail.com>
>
>
> Dear Mauro Carvalho Chehab
> My name is YongSu Yoo
>
> I found an incorrect logic at the function of dvb_ringbuffer_pkt_next in
> /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c.
> I correct the bug like the attached file (
> 0001-media-dvb_ringbuffer-Fix-a-bug-in-dvb_ringbuffer.c.patch ).
> Please check the attached file.
>
> Thank you

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Kieran Bingham]

Hi YongSu Yoo,

Quoting 유용수 (2022-06-22 12:39:20)
> Dear Alls
> 
> How is my kernel modification going ?
> Can you check my kernel modification ?
> and give me the feedback ?

Your patch needs to be sent inline, not as an attachement. It has a
wrong mime type or such so I haven't even been able to open it yet.


There should be helpful information in:
 https://www.kernel.org/doc/html/latest/process/submitting-patches.html

--
Kieran

> 
> 2022년 6월 1일 (수) 오후 9:54, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >
> > Dear Alls
> >
> > Can you check the Below E-mail ?
> > Can you share how is the below patch going ?
> >
> > ---------- Forwarded message ---------
> > 보낸사람: 유용수 <yongsuyoo0215@gmail.com>
> > Date: 2022년 5월 22일 (일) 오후 3:36
> > Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> > To: <mchehab@kernel.org>
> > Cc: <0215yys@hanmail.net>, <yongsu.yoo@lge.com>, 유용수 <yongsuyoo0215@gmail.com>
> >
> >
> > Dear Mauro Carvalho Chehab
> > My name is YongSu Yoo
> >
> > I found an incorrect logic at the function of dvb_ringbuffer_pkt_next in
> > /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c.
> > I correct the bug like the attached file (
> > 0001-media-dvb_ringbuffer-Fix-a-bug-in-dvb_ringbuffer.c.patch ).
> > Please check the attached file.
> >
> > Thank you

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Hans Petter Selasky]

Hi Kieran,

The consumed variable should not be negative. This bug has been there 
since the beginning of the GIT at Linux from what I can see.

+1

--HPS

 From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
From: YongSu Yoo <yongsuyoo0215@gmail.com>
Date: Sun, 22 May 2022 04:53:12 +0000
Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>

The function dvb_ringbuffer_pkt_next in
/linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
which searches the idx of the next valid packet in the ring
buffer of the ca->slot_info[slot].rx_buffer at
/linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
has the following problem.
In calculating the amounts of the consumed address of the ring
buffer, if the read address(rbuf->pread) of the ring buffer is
smaller than the idx, the amounts of the searched address
should be (idx - rbuf->pread),
whereas if the read address(rbuf->pread) of the ring buffer is
larger than the idx, the amounts of the consumed address should
be (idx - rbuf->pread + rbug->size). But there exists an
incorrect logic that the rbug-size was not properly added on
(idx - rbug->pread) in the later case. With this commit, we
fixed this bug.
---
  drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c 
b/drivers/media/dvb-core/dvb_ringbuffer.c
index d1d471af0636..7d4558de8e83 100644
--- a/drivers/media/dvb-core/dvb_ringbuffer.c
+++ b/drivers/media/dvb-core/dvb_ringbuffer.c
@@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct 
dvb_ringbuffer *rbuf, size_t idx, size_t*
  		idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
  	}

-	consumed = (idx - rbuf->pread) % rbuf->size;
+	consumed = (idx - rbuf->pread);
+	if (consumed < 0)
+		consumed += rbuf->size;

  	while((dvb_ringbuffer_avail(rbuf) - consumed) > 
DVB_RINGBUFFER_PKTHDRSIZE) {

-- 
2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear Kieran Bingham

I sent E-mail again by Linux terminal by using the command " ... git
send-email .."
I believe that you will surely get the diff file.

Thank you

2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
>
> Hi Kieran,
>
> The consumed variable should not be negative. This bug has been there
> since the beginning of the GIT at Linux from what I can see.
>
> +1
>
> --HPS
>
>  From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
> From: YongSu Yoo <yongsuyoo0215@gmail.com>
> Date: Sun, 22 May 2022 04:53:12 +0000
> Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
>
> Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
>
> The function dvb_ringbuffer_pkt_next in
> /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> which searches the idx of the next valid packet in the ring
> buffer of the ca->slot_info[slot].rx_buffer at
> /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> has the following problem.
> In calculating the amounts of the consumed address of the ring
> buffer, if the read address(rbuf->pread) of the ring buffer is
> smaller than the idx, the amounts of the searched address
> should be (idx - rbuf->pread),
> whereas if the read address(rbuf->pread) of the ring buffer is
> larger than the idx, the amounts of the consumed address should
> be (idx - rbuf->pread + rbug->size). But there exists an
> incorrect logic that the rbug-size was not properly added on
> (idx - rbug->pread) in the later case. With this commit, we
> fixed this bug.
> ---
>   drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
> b/drivers/media/dvb-core/dvb_ringbuffer.c
> index d1d471af0636..7d4558de8e83 100644
> --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
> dvb_ringbuffer *rbuf, size_t idx, size_t*
>                 idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
>         }
>
> -       consumed = (idx - rbuf->pread) % rbuf->size;
> +       consumed = (idx - rbuf->pread);
> +       if (consumed < 0)
> +               consumed += rbuf->size;
>
>         while((dvb_ringbuffer_avail(rbuf) - consumed) >
> DVB_RINGBUFFER_PKTHDRSIZE) {
>
> --
> 2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear All.
Too much time has already passed since I first sent this Email.
Can you share how this patch is going ?

2022년 6월 23일 (목) 오후 6:11, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>
> Dear Kieran Bingham
>
> I sent E-mail again by Linux terminal by using the command " ... git
> send-email .."
> I believe that you will surely get the diff file.
>
> Thank you
>
> 2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
> >
> > Hi Kieran,
> >
> > The consumed variable should not be negative. This bug has been there
> > since the beginning of the GIT at Linux from what I can see.
> >
> > +1
> >
> > --HPS
> >
> >  From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
> > From: YongSu Yoo <yongsuyoo0215@gmail.com>
> > Date: Sun, 22 May 2022 04:53:12 +0000
> > Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> >
> > Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
> >
> > The function dvb_ringbuffer_pkt_next in
> > /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> > which searches the idx of the next valid packet in the ring
> > buffer of the ca->slot_info[slot].rx_buffer at
> > /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> > has the following problem.
> > In calculating the amounts of the consumed address of the ring
> > buffer, if the read address(rbuf->pread) of the ring buffer is
> > smaller than the idx, the amounts of the searched address
> > should be (idx - rbuf->pread),
> > whereas if the read address(rbuf->pread) of the ring buffer is
> > larger than the idx, the amounts of the consumed address should
> > be (idx - rbuf->pread + rbug->size). But there exists an
> > incorrect logic that the rbug-size was not properly added on
> > (idx - rbug->pread) in the later case. With this commit, we
> > fixed this bug.
> > ---
> >   drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
> > b/drivers/media/dvb-core/dvb_ringbuffer.c
> > index d1d471af0636..7d4558de8e83 100644
> > --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> > +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> > @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
> > dvb_ringbuffer *rbuf, size_t idx, size_t*
> >                 idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
> >         }
> >
> > -       consumed = (idx - rbuf->pread) % rbuf->size;
> > +       consumed = (idx - rbuf->pread);
> > +       if (consumed < 0)
> > +               consumed += rbuf->size;
> >
> >         while((dvb_ringbuffer_avail(rbuf) - consumed) >
> > DVB_RINGBUFFER_PKTHDRSIZE) {
> >
> > --
> > 2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear All,

Can you pay your attention to this patch ?
This is a very simple modification.
But, too much time has already passed since I sent the first E-mail.

2022년 8월 3일 (수) 오후 9:04, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>
> Dear All.
> Too much time has already passed since I first sent this Email.
> Can you share how this patch is going ?
>
> 2022년 6월 23일 (목) 오후 6:11, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >
> > Dear Kieran Bingham
> >
> > I sent E-mail again by Linux terminal by using the command " ... git
> > send-email .."
> > I believe that you will surely get the diff file.
> >
> > Thank you
> >
> > 2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
> > >
> > > Hi Kieran,
> > >
> > > The consumed variable should not be negative. This bug has been there
> > > since the beginning of the GIT at Linux from what I can see.
> > >
> > > +1
> > >
> > > --HPS
> > >
> > >  From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
> > > From: YongSu Yoo <yongsuyoo0215@gmail.com>
> > > Date: Sun, 22 May 2022 04:53:12 +0000
> > > Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> > >
> > > Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
> > >
> > > The function dvb_ringbuffer_pkt_next in
> > > /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> > > which searches the idx of the next valid packet in the ring
> > > buffer of the ca->slot_info[slot].rx_buffer at
> > > /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> > > has the following problem.
> > > In calculating the amounts of the consumed address of the ring
> > > buffer, if the read address(rbuf->pread) of the ring buffer is
> > > smaller than the idx, the amounts of the searched address
> > > should be (idx - rbuf->pread),
> > > whereas if the read address(rbuf->pread) of the ring buffer is
> > > larger than the idx, the amounts of the consumed address should
> > > be (idx - rbuf->pread + rbug->size). But there exists an
> > > incorrect logic that the rbug-size was not properly added on
> > > (idx - rbug->pread) in the later case. With this commit, we
> > > fixed this bug.
> > > ---
> > >   drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
> > > b/drivers/media/dvb-core/dvb_ringbuffer.c
> > > index d1d471af0636..7d4558de8e83 100644
> > > --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> > > +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> > > @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
> > > dvb_ringbuffer *rbuf, size_t idx, size_t*
> > >                 idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
> > >         }
> > >
> > > -       consumed = (idx - rbuf->pread) % rbuf->size;
> > > +       consumed = (idx - rbuf->pread);
> > > +       if (consumed < 0)
> > > +               consumed += rbuf->size;
> > >
> > >         while((dvb_ringbuffer_avail(rbuf) - consumed) >
> > > DVB_RINGBUFFER_PKTHDRSIZE) {
> > >
> > > --
> > > 2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear All,

Can you share how this patch is going ?

2022년 8월 18일 (목) 오후 9:55, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>
> Dear All,
>
> Can you pay your attention to this patch ?
> This is a very simple modification.
> But, too much time has already passed since I sent the first E-mail.
>
> 2022년 8월 3일 (수) 오후 9:04, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >
> > Dear All.
> > Too much time has already passed since I first sent this Email.
> > Can you share how this patch is going ?
> >
> > 2022년 6월 23일 (목) 오후 6:11, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> > >
> > > Dear Kieran Bingham
> > >
> > > I sent E-mail again by Linux terminal by using the command " ... git
> > > send-email .."
> > > I believe that you will surely get the diff file.
> > >
> > > Thank you
> > >
> > > 2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
> > > >
> > > > Hi Kieran,
> > > >
> > > > The consumed variable should not be negative. This bug has been there
> > > > since the beginning of the GIT at Linux from what I can see.
> > > >
> > > > +1
> > > >
> > > > --HPS
> > > >
> > > >  From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
> > > > From: YongSu Yoo <yongsuyoo0215@gmail.com>
> > > > Date: Sun, 22 May 2022 04:53:12 +0000
> > > > Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> > > >
> > > > Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
> > > >
> > > > The function dvb_ringbuffer_pkt_next in
> > > > /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> > > > which searches the idx of the next valid packet in the ring
> > > > buffer of the ca->slot_info[slot].rx_buffer at
> > > > /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> > > > has the following problem.
> > > > In calculating the amounts of the consumed address of the ring
> > > > buffer, if the read address(rbuf->pread) of the ring buffer is
> > > > smaller than the idx, the amounts of the searched address
> > > > should be (idx - rbuf->pread),
> > > > whereas if the read address(rbuf->pread) of the ring buffer is
> > > > larger than the idx, the amounts of the consumed address should
> > > > be (idx - rbuf->pread + rbug->size). But there exists an
> > > > incorrect logic that the rbug-size was not properly added on
> > > > (idx - rbug->pread) in the later case. With this commit, we
> > > > fixed this bug.
> > > > ---
> > > >   drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
> > > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
> > > > b/drivers/media/dvb-core/dvb_ringbuffer.c
> > > > index d1d471af0636..7d4558de8e83 100644
> > > > --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> > > > +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> > > > @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
> > > > dvb_ringbuffer *rbuf, size_t idx, size_t*
> > > >                 idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
> > > >         }
> > > >
> > > > -       consumed = (idx - rbuf->pread) % rbuf->size;
> > > > +       consumed = (idx - rbuf->pread);
> > > > +       if (consumed < 0)
> > > > +               consumed += rbuf->size;
> > > >
> > > >         while((dvb_ringbuffer_avail(rbuf) - consumed) >
> > > > DVB_RINGBUFFER_PKTHDRSIZE) {
> > > >
> > > > --
> > > > 2.17.1

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Hans Petter Selasky]

On 8/31/22 15:45, 유용수 wrote:
> Dear All,
> 
> Can you share how this patch is going ?
> 
> 2022년 8월 18일 (목) 오후 9:55, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>>
>> Dear All,
>>
>> Can you pay your attention to this patch ?
>> This is a very simple modification.
>> But, too much time has already passed since I sent the first E-mail.
>>
>> 2022년 8월 3일 (수) 오후 9:04, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>>>
>>> Dear All.
>>> Too much time has already passed since I first sent this Email.
>>> Can you share how this patch is going ?
>>>
>>> 2022년 6월 23일 (목) 오후 6:11, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
>>>>
>>>> Dear Kieran Bingham
>>>>
>>>> I sent E-mail again by Linux terminal by using the command " ... git
>>>> send-email .."
>>>> I believe that you will surely get the diff file.
>>>>
>>>> Thank you
>>>>
>>>> 2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
>>>>>
>>>>> Hi Kieran,
>>>>>
>>>>> The consumed variable should not be negative. This bug has been there
>>>>> since the beginning of the GIT at Linux from what I can see.
>>>>>
>>>>> +1
>>>>>
>>>>> --HPS
>>>>>
>>>>>   From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
>>>>> From: YongSu Yoo <yongsuyoo0215@gmail.com>
>>>>> Date: Sun, 22 May 2022 04:53:12 +0000
>>>>> Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
>>>>>
>>>>> Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
>>>>>
>>>>> The function dvb_ringbuffer_pkt_next in
>>>>> /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
>>>>> which searches the idx of the next valid packet in the ring
>>>>> buffer of the ca->slot_info[slot].rx_buffer at
>>>>> /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
>>>>> has the following problem.
>>>>> In calculating the amounts of the consumed address of the ring
>>>>> buffer, if the read address(rbuf->pread) of the ring buffer is
>>>>> smaller than the idx, the amounts of the searched address
>>>>> should be (idx - rbuf->pread),
>>>>> whereas if the read address(rbuf->pread) of the ring buffer is
>>>>> larger than the idx, the amounts of the consumed address should
>>>>> be (idx - rbuf->pread + rbug->size). But there exists an
>>>>> incorrect logic that the rbug-size was not properly added on
>>>>> (idx - rbug->pread) in the later case. With this commit, we
>>>>> fixed this bug.
>>>>> ---
>>>>>    drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
>>>>> b/drivers/media/dvb-core/dvb_ringbuffer.c
>>>>> index d1d471af0636..7d4558de8e83 100644
>>>>> --- a/drivers/media/dvb-core/dvb_ringbuffer.c
>>>>> +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
>>>>> @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
>>>>> dvb_ringbuffer *rbuf, size_t idx, size_t*
>>>>>                  idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
>>>>>          }
>>>>>
>>>>> -       consumed = (idx - rbuf->pread) % rbuf->size;
>>>>> +       consumed = (idx - rbuf->pread);
>>>>> +       if (consumed < 0)
>>>>> +               consumed += rbuf->size;
>>>>>
>>>>>          while((dvb_ringbuffer_avail(rbuf) - consumed) >
>>>>> DVB_RINGBUFFER_PKTHDRSIZE) {
>>>>>
>>>>> --
>>>>> 2.17.1

Maybe it will help if you write a test C-program to show the bad numbers 
computed?

--HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear Hans Petter Selasky

Thank you for your sincere replyment.
But I cannot understand what you mean.
Can you kindly explain what kind of C-program do I have to write ?

2022년 9월 9일 (금) 오후 11:46, Hans Petter Selasky <hps@selasky.org>님이 작성:
>
> On 8/31/22 15:45, 유용수 wrote:
> > Dear All,
> >
> > Can you share how this patch is going ?
> >
> > 2022년 8월 18일 (목) 오후 9:55, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >>
> >> Dear All,
> >>
> >> Can you pay your attention to this patch ?
> >> This is a very simple modification.
> >> But, too much time has already passed since I sent the first E-mail.
> >>
> >> 2022년 8월 3일 (수) 오후 9:04, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >>>
> >>> Dear All.
> >>> Too much time has already passed since I first sent this Email.
> >>> Can you share how this patch is going ?
> >>>
> >>> 2022년 6월 23일 (목) 오후 6:11, 유용수 <yongsuyoo0215@gmail.com>님이 작성:
> >>>>
> >>>> Dear Kieran Bingham
> >>>>
> >>>> I sent E-mail again by Linux terminal by using the command " ... git
> >>>> send-email .."
> >>>> I believe that you will surely get the diff file.
> >>>>
> >>>> Thank you
> >>>>
> >>>> 2022년 6월 22일 (수) 오후 10:12, Hans Petter Selasky <hps@selasky.org>님이 작성:
> >>>>>
> >>>>> Hi Kieran,
> >>>>>
> >>>>> The consumed variable should not be negative. This bug has been there
> >>>>> since the beginning of the GIT at Linux from what I can see.
> >>>>>
> >>>>> +1
> >>>>>
> >>>>> --HPS
> >>>>>
> >>>>>   From 108c6acb2cc4bc4314b96f6f254a04b2873a096c Mon Sep 17 00:00:00 2001
> >>>>> From: YongSu Yoo <yongsuyoo0215@gmail.com>
> >>>>> Date: Sun, 22 May 2022 04:53:12 +0000
> >>>>> Subject: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c
> >>>>>
> >>>>> Signed-off-by:Yongsu Yoo <yongsuyoo0215@gmail.com>
> >>>>>
> >>>>> The function dvb_ringbuffer_pkt_next in
> >>>>> /linux-next/drviers/media/dvb-core/dvb_ringbuffer.c,
> >>>>> which searches the idx of the next valid packet in the ring
> >>>>> buffer of the ca->slot_info[slot].rx_buffer at
> >>>>> /linux-next/drivers/media/dvb-core/dvb_ca_en50221.c,
> >>>>> has the following problem.
> >>>>> In calculating the amounts of the consumed address of the ring
> >>>>> buffer, if the read address(rbuf->pread) of the ring buffer is
> >>>>> smaller than the idx, the amounts of the searched address
> >>>>> should be (idx - rbuf->pread),
> >>>>> whereas if the read address(rbuf->pread) of the ring buffer is
> >>>>> larger than the idx, the amounts of the consumed address should
> >>>>> be (idx - rbuf->pread + rbug->size). But there exists an
> >>>>> incorrect logic that the rbug-size was not properly added on
> >>>>> (idx - rbug->pread) in the later case. With this commit, we
> >>>>> fixed this bug.
> >>>>> ---
> >>>>>    drivers/media/dvb-core/dvb_ringbuffer.c | 4 +++-
> >>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/media/dvb-core/dvb_ringbuffer.c
> >>>>> b/drivers/media/dvb-core/dvb_ringbuffer.c
> >>>>> index d1d471af0636..7d4558de8e83 100644
> >>>>> --- a/drivers/media/dvb-core/dvb_ringbuffer.c
> >>>>> +++ b/drivers/media/dvb-core/dvb_ringbuffer.c
> >>>>> @@ -335,7 +335,9 @@ ssize_t dvb_ringbuffer_pkt_next(struct
> >>>>> dvb_ringbuffer *rbuf, size_t idx, size_t*
> >>>>>                  idx = (idx + curpktlen + DVB_RINGBUFFER_PKTHDRSIZE) % rbuf->size;
> >>>>>          }
> >>>>>
> >>>>> -       consumed = (idx - rbuf->pread) % rbuf->size;
> >>>>> +       consumed = (idx - rbuf->pread);
> >>>>> +       if (consumed < 0)
> >>>>> +               consumed += rbuf->size;
> >>>>>
> >>>>>          while((dvb_ringbuffer_avail(rbuf) - consumed) >
> >>>>> DVB_RINGBUFFER_PKTHDRSIZE) {
> >>>>>
> >>>>> --
> >>>>> 2.17.1
>
> Maybe it will help if you write a test C-program to show the bad numbers
> computed?
>
> --HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Hans Petter Selasky]

Hi Mauro and YongSu,

Answering my own question: The reason nobody has triggered this yet, is 
because the buffer size used is power of two. Because unsigned modulus 
is used, the result becomes correct. See below. But if non-power of two 
ring-buffer is used, then the result becomes incorrect. There is no 
block for non-power of two sized buffers. See:

https://github.com/search?q=dvb_set_pesfilter&type=code

cat << EOF > testX.c
#include <stdio.h>

int
main()
{
int consumed_old;
int consumed_fix;
size_t idx = 3;
ssize_t pread = 15;
ssize_t size = 256;

consumed_old = (idx - pread) % size;

consumed_fix = (idx - pread);
if (consumed_fix < 0)
consumed_fix += size;

printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);

size = 254;

consumed_old = (idx - pread) % size;

consumed_fix = (idx - pread);
if (consumed_fix < 0)
consumed_fix += size;

printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);

return (0);
}
EOF

cc testX.c && ./a.out
old=244 new=244 size=256
old=244 new=242 size=254

So either push the suggested fix, or block non-power of two buffer sizes!

Best regards,
--HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[유용수]

Dear Hans Petter Selasky

I understood your points.
Thank you for your kind explanation
I found that the buffer size is 65535 like below source code.
The 65535 is not the power of two.
So it can still be a problem.
...
#define RX_BUFFER_SIZE 65535
...
rxbuf = vmalloc(RX_BUFFER_SIZE);
...
dvb_ringbuffer_init(&ca->slot_info[slot].rx_buffer, rxbuf, RX_BUFFER_SIZE);
}
...

2022년 9월 12일 (월) 오후 9:36, Hans Petter Selasky <hps@selasky.org>님이 작성:

>
> Hi Mauro and YongSu,
>
> Answering my own question: The reason nobody has triggered this yet, is
> because the buffer size used is power of two. Because unsigned modulus
> is used, the result becomes correct. See below. But if non-power of two
> ring-buffer is used, then the result becomes incorrect. There is no
> block for non-power of two sized buffers. See:
>
> https://github.com/search?q=dvb_set_pesfilter&type=code
>
> cat << EOF > testX.c
> #include <stdio.h>
>
> int
> main()
> {
> int consumed_old;
> int consumed_fix;
> size_t idx = 3;
> ssize_t pread = 15;
> ssize_t size = 256;
>
> consumed_old = (idx - pread) % size;
>
> consumed_fix = (idx - pread);
> if (consumed_fix < 0)
> consumed_fix += size;
>
> printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);
>
> size = 254;
>
> consumed_old = (idx - pread) % size;
>
> consumed_fix = (idx - pread);
> if (consumed_fix < 0)
> consumed_fix += size;
>
> printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);
>
> return (0);
> }
> EOF
>
> cc testX.c && ./a.out
> old=244 new=244 size=256
> old=244 new=242 size=254
>
> So either push the suggested fix, or block non-power of two buffer sizes!
>
> Best regards,
> --HPS

Re: [PATCH] media: dvb_ringbuffer : Fix a bug in dvb_ringbuffer.c

[Hans Petter Selasky]

Hi YongSu,

I'm stripping "linux-kernel@vger.kernel.org". Hopefully that will help. 
Developers may receive hundreds of e-mails every day, and CC'ing too 
many lists may cause the e-mail to get ignored.

Mauro, CC'ed: Can we have a comment on this issue from some Linux Media 
developers having commit access?

--HPS

On 9/13/22 10:36, 유용수 wrote:
> Dear Hans Petter Selasky
> 
> I understood your points.
> Thank you for your kind explanation
> I found that the buffer size is 65535 like below source code.
> The 65535 is not the power of two.
> So it can still be a problem.
> ...
> #define RX_BUFFER_SIZE 65535
> ...
> rxbuf = vmalloc(RX_BUFFER_SIZE);
> ...
> dvb_ringbuffer_init(&ca->slot_info[slot].rx_buffer, rxbuf, RX_BUFFER_SIZE);
> }
> ...
> 
> 2022년 9월 12일 (월) 오후 9:36, Hans Petter Selasky <hps@selasky.org>님이 작성:
> 
>>
>> Hi Mauro and YongSu,
>>
>> Answering my own question: The reason nobody has triggered this yet, is
>> because the buffer size used is power of two. Because unsigned modulus
>> is used, the result becomes correct. See below. But if non-power of two
>> ring-buffer is used, then the result becomes incorrect. There is no
>> block for non-power of two sized buffers. See:
>>
>> https://github.com/search?q=dvb_set_pesfilter&type=code
>>
>> cat << EOF > testX.c
>> #include <stdio.h>
>>
>> int
>> main()
>> {
>> int consumed_old;
>> int consumed_fix;
>> size_t idx = 3;
>> ssize_t pread = 15;
>> ssize_t size = 256;
>>
>> consumed_old = (idx - pread) % size;
>>
>> consumed_fix = (idx - pread);
>> if (consumed_fix < 0)
>> consumed_fix += size;
>>
>> printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);
>>
>> size = 254;
>>
>> consumed_old = (idx - pread) % size;
>>
>> consumed_fix = (idx - pread);
>> if (consumed_fix < 0)
>> consumed_fix += size;
>>
>> printf("old=%d new=%d size=%zd\n", consumed_old, consumed_fix, size);
>>
>> return (0);
>> }
>> EOF
>>
>> cc testX.c && ./a.out
>> old=244 new=244 size=256
>> old=244 new=242 size=254
>>
>> So either push the suggested fix, or block non-power of two buffer sizes!
>>
>> Best regards,
>> --HPS