All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>,
	linux-media@vger.kernel.org,
	linux1394-devel@lists.sourceforge.net,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] [media] firewire: firedtv-avc: potential buffer overflow
Date: Tue, 09 Sep 2014 08:36:27 +0000	[thread overview]
Message-ID: <20140909083627.GI6549@mwanda> (raw)
In-Reply-To: <20140908144033.42a0762d@kant>

On Mon, Sep 08, 2014 at 02:40:33PM +0200, Stefan Richter wrote:
> On Sep 08 Stefan Richter wrote:
> > On Sep 08 Dan Carpenter wrote:
> > > "program_info_length" is user controlled and can go up to 4095.  The
> > > operand[] array has 509 bytes so we need to add a limit here to prevent
> > > buffer overflows.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Reviewed-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > 
> > Thank you.
> 
> Oops, that was a bit too quick.  After the memcpy() accesses which you
> protect, there are another four bytes written, still without checking
> the bounds.

Thanks for catching that.  I'll send a v2 soon.

Btw, my static checker complains about the remaining memcpy() in this
file:

drivers/media/firewire/firedtv-avc.c:1310 avc_ca_get_mmi() error: '*len' from user is not capped properly

This static checker warning has a lot of false positives.  I looked at
the code for a long time but couldn't figure out why it thinks "*len"
is untrusted.  I also wasn't totally sure that it was safe?

regards,
dan carpenter


WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>,
	linux-media@vger.kernel.org,
	linux1394-devel@lists.sourceforge.net,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] [media] firewire: firedtv-avc: potential buffer overflow
Date: Tue, 9 Sep 2014 11:36:27 +0300	[thread overview]
Message-ID: <20140909083627.GI6549@mwanda> (raw)
In-Reply-To: <20140908144033.42a0762d@kant>

On Mon, Sep 08, 2014 at 02:40:33PM +0200, Stefan Richter wrote:
> On Sep 08 Stefan Richter wrote:
> > On Sep 08 Dan Carpenter wrote:
> > > "program_info_length" is user controlled and can go up to 4095.  The
> > > operand[] array has 509 bytes so we need to add a limit here to prevent
> > > buffer overflows.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > Reviewed-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> > 
> > Thank you.
> 
> Oops, that was a bit too quick.  After the memcpy() accesses which you
> protect, there are another four bytes written, still without checking
> the bounds.

Thanks for catching that.  I'll send a v2 soon.

Btw, my static checker complains about the remaining memcpy() in this
file:

drivers/media/firewire/firedtv-avc.c:1310 avc_ca_get_mmi() error: '*len' from user is not capped properly

This static checker warning has a lot of false positives.  I looked at
the code for a long time but couldn't figure out why it thinks "*len"
is untrusted.  I also wasn't totally sure that it was safe?

regards,
dan carpenter


  reply	other threads:[~2014-09-09  8:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-08 11:18 [patch] [media] firewire: firedtv-avc: potential buffer overflow Dan Carpenter
2014-09-08 11:18 ` Dan Carpenter
2014-09-08 12:05 ` Stefan Richter
2014-09-08 12:05   ` Stefan Richter
2014-09-08 12:40   ` Stefan Richter
2014-09-08 12:40     ` Stefan Richter
2014-09-09  8:36     ` Dan Carpenter [this message]
2014-09-09  8:36       ` Dan Carpenter
2014-09-09 12:11     ` [patch v2] " Dan Carpenter
2014-09-09 12:11       ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140909083627.GI6549@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    --cc=m.chehab@samsung.com \
    --cc=stefanr@s5r6.in-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.