All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: santosh nayak <santoshprasadnayak@gmail.com>,
	netfilter@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	davem@davemloft.net, bart.de.schuymer@pandora.be,
	netfilter-devel@vger.kernel.org, shemminger@vyatta.com
Subject: Re: [Bridge] [PATCH 1/3] netfilter: Fix copy_to_user too small size parametre.
Date: Thu, 1 Mar 2012 14:06:37 +0100	[thread overview]
Message-ID: <20120301130637.GB7429@1984> (raw)
In-Reply-To: <20120301113736.GE22598@mwanda>

On Thu, Mar 01, 2012 at 02:37:36PM +0300, Dan Carpenter wrote:
> On Thu, Mar 01, 2012 at 11:18:09AM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Mar 01, 2012 at 02:46:30PM +0530, santosh nayak wrote:
> > > From: Santosh Nayak <santoshprasadnayak@gmail.com>
> > > 
> > > While copying to userspace, the size of source is 29byte where as
> > > size parametre is 32 byte.  Its leaking extra-information from
> > > kernel space to user space.
> > > Replace EBT_FUNCTION_MAXNAMELEN by XT_EXTENSION_MAXNAMELEN.
> > 
> > There's no information leak.
> > 
> 
> Where do we clear "m"? 
> 
> include/linux/netfilter/x_tables.h
>    287  struct xt_match {
>    288          struct list_head list;
>    289  
>    290          const char name[XT_EXTENSION_MAXNAMELEN];
>    291          u_int8_t revision;
>    292  
> 
> There is a 2 byte holes here between "revision" and "match()".  We
> copy three bytes past the end of name, so we include revision and
> the hole.
> 
> But maybe we memset it somewhere?  I'm not sure.

xt_match instances are declared as static for each module so it's
allocated in the BSS (already zeroed), is that what you mean?

>    293          /* Return true or false: return FALSE and set *hotdrop = 1 to
>    294             force immediate packet drop. */
>    295          /* Arguments changed since 2.6.9, as this must now handle
>    296             non-linear skb, using skb_header_pointer and
>    297             skb_ip_make_writable. */
>    298          bool (*match)(const struct sk_buff *skb,
>    299                        struct xt_action_param *);
> 
> regards,
> dan carpenter
> 



WARNING: multiple messages have this Message-ID (diff)
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: santosh nayak <santoshprasadnayak@gmail.com>,
	bart.de.schuymer@pandora.be, kaber@trash.net,
	shemminger@vyatta.com, davem@davemloft.net,
	netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org,
	coreteam@netfilter.org, bridge@lists.linux-foundation.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/3] netfilter: Fix copy_to_user too small size parametre.
Date: Thu, 01 Mar 2012 13:06:37 +0000	[thread overview]
Message-ID: <20120301130637.GB7429@1984> (raw)
In-Reply-To: <20120301113736.GE22598@mwanda>

On Thu, Mar 01, 2012 at 02:37:36PM +0300, Dan Carpenter wrote:
> On Thu, Mar 01, 2012 at 11:18:09AM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Mar 01, 2012 at 02:46:30PM +0530, santosh nayak wrote:
> > > From: Santosh Nayak <santoshprasadnayak@gmail.com>
> > > 
> > > While copying to userspace, the size of source is 29byte where as
> > > size parametre is 32 byte.  Its leaking extra-information from
> > > kernel space to user space.
> > > Replace EBT_FUNCTION_MAXNAMELEN by XT_EXTENSION_MAXNAMELEN.
> > 
> > There's no information leak.
> > 
> 
> Where do we clear "m"? 
> 
> include/linux/netfilter/x_tables.h
>    287  struct xt_match {
>    288          struct list_head list;
>    289  
>    290          const char name[XT_EXTENSION_MAXNAMELEN];
>    291          u_int8_t revision;
>    292  
> 
> There is a 2 byte holes here between "revision" and "match()".  We
> copy three bytes past the end of name, so we include revision and
> the hole.
> 
> But maybe we memset it somewhere?  I'm not sure.

xt_match instances are declared as static for each module so it's
allocated in the BSS (already zeroed), is that what you mean?

>    293          /* Return true or false: return FALSE and set *hotdrop = 1 to
>    294             force immediate packet drop. */
>    295          /* Arguments changed since 2.6.9, as this must now handle
>    296             non-linear skb, using skb_header_pointer and
>    297             skb_ip_make_writable. */
>    298          bool (*match)(const struct sk_buff *skb,
>    299                        struct xt_action_param *);
> 
> regards,
> dan carpenter
> 



WARNING: multiple messages have this Message-ID (diff)
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: santosh nayak <santoshprasadnayak@gmail.com>,
	bart.de.schuymer@pandora.be, kaber@trash.net,
	shemminger@vyatta.com, davem@davemloft.net,
	netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org,
	coreteam@netfilter.org, bridge@lists.linux-foundation.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH 1/3] netfilter: Fix copy_to_user too small size parametre.
Date: Thu, 1 Mar 2012 14:06:37 +0100	[thread overview]
Message-ID: <20120301130637.GB7429@1984> (raw)
In-Reply-To: <20120301113736.GE22598@mwanda>

On Thu, Mar 01, 2012 at 02:37:36PM +0300, Dan Carpenter wrote:
> On Thu, Mar 01, 2012 at 11:18:09AM +0100, Pablo Neira Ayuso wrote:
> > On Thu, Mar 01, 2012 at 02:46:30PM +0530, santosh nayak wrote:
> > > From: Santosh Nayak <santoshprasadnayak@gmail.com>
> > > 
> > > While copying to userspace, the size of source is 29byte where as
> > > size parametre is 32 byte.  Its leaking extra-information from
> > > kernel space to user space.
> > > Replace EBT_FUNCTION_MAXNAMELEN by XT_EXTENSION_MAXNAMELEN.
> > 
> > There's no information leak.
> > 
> 
> Where do we clear "m"? 
> 
> include/linux/netfilter/x_tables.h
>    287  struct xt_match {
>    288          struct list_head list;
>    289  
>    290          const char name[XT_EXTENSION_MAXNAMELEN];
>    291          u_int8_t revision;
>    292  
> 
> There is a 2 byte holes here between "revision" and "match()".  We
> copy three bytes past the end of name, so we include revision and
> the hole.
> 
> But maybe we memset it somewhere?  I'm not sure.

xt_match instances are declared as static for each module so it's
allocated in the BSS (already zeroed), is that what you mean?

>    293          /* Return true or false: return FALSE and set *hotdrop = 1 to
>    294             force immediate packet drop. */
>    295          /* Arguments changed since 2.6.9, as this must now handle
>    296             non-linear skb, using skb_header_pointer and
>    297             skb_ip_make_writable. */
>    298          bool (*match)(const struct sk_buff *skb,
>    299                        struct xt_action_param *);
> 
> regards,
> dan carpenter
> 


  reply	other threads:[~2012-03-01 13:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-01  9:16 [Bridge] [PATCH 1/3] netfilter: Fix copy_to_user too small size parametre santosh nayak
2012-03-01  9:28 ` santosh nayak
2012-03-01  9:16 ` santosh nayak
2012-03-01 10:18 ` [Bridge] " Pablo Neira Ayuso
2012-03-01 10:18   ` Pablo Neira Ayuso
2012-03-01 10:18   ` Pablo Neira Ayuso
2012-03-01 10:45   ` [Bridge] " santosh prasad nayak
2012-03-01 10:57     ` santosh prasad nayak
2012-03-01 10:45     ` santosh prasad nayak
2012-03-01 13:03     ` [Bridge] " Pablo Neira Ayuso
2012-03-01 13:03       ` Pablo Neira Ayuso
2012-03-01 13:03       ` Pablo Neira Ayuso
2012-03-01 13:51       ` [Bridge] " santosh prasad nayak
2012-03-01 13:52         ` santosh prasad nayak
2012-03-01 13:51         ` santosh prasad nayak
2012-03-01 11:37   ` [Bridge] " Dan Carpenter
2012-03-01 11:37     ` Dan Carpenter
2012-03-01 11:37     ` Dan Carpenter
2012-03-01 13:06     ` Pablo Neira Ayuso [this message]
2012-03-01 13:06       ` Pablo Neira Ayuso
2012-03-01 13:06       ` Pablo Neira Ayuso
2012-03-01 13:13       ` [Bridge] " Dan Carpenter
2012-03-01 13:13         ` Dan Carpenter
2012-03-01 13:13         ` Dan Carpenter
2012-03-01 11:31 ` [Bridge] " Dan Carpenter
2012-03-01 11:31   ` Dan Carpenter
2012-03-01 11:31   ` Dan Carpenter
2012-03-01 15:00 ` walter harms

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=20120301130637.GB7429@1984 \
    --to=pablo@netfilter.org \
    --cc=bart.de.schuymer@pandora.be \
    --cc=bridge@lists.linux-foundation.org \
    --cc=coreteam@netfilter.org \
    --cc=dan.carpenter@oracle.com \
    --cc=davem@davemloft.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=netfilter@vger.kernel.org \
    --cc=santoshprasadnayak@gmail.com \
    --cc=shemminger@vyatta.com \
    /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.