Re: CVS commit: src

2014-04-21 Thread Maxime Villard
Le 21/04/2014 01:46, Taylor R Campbell a écrit :
 
From: Maxime Villard m...@netbsd.org
Date: Wed, 16 Apr 2014 18:55:20 +
 
An (un)privileged user can easily make the kernel dereference a NULL
pointer.
 
The kernel allows 'data' to be NULL; it's the fs's responsibility to
ensure that it isn't NULL (if the fs actually needs data).
 
 In most cases of the changes you made, there is already a test for the
 length of the data buffer.  Is this not guaranteed to be zero if data
 is null?  It seems to me that the length test ought to suffice, and if
 anything the null pointer test should be an assertion, not a check.
 

Not at all. 'data' and 'data_len' come from userpace. A user can set data
to NULL and data_len to a value high enough to bypass the data_len check.

I've already demonstrated that to security-alert@.


Re: CVS commit: src

2014-04-21 Thread Taylor R Campbell
   Date: Mon, 21 Apr 2014 08:20:22 +0200
   From: Maxime Villard m...@m00nbsd.net

   Le 21/04/2014 01:46, Taylor R Campbell a écrit :
In most cases of the changes you made, there is already a test for the
length of the data buffer.  Is this not guaranteed to be zero if data
is null?  It seems to me that the length test ought to suffice, and if
anything the null pointer test should be an assertion, not a check.

   Not at all. 'data' and 'data_len' come from userpace. A user can set data
   to NULL and data_len to a value high enough to bypass the data_len check.

If a user passes in null data and nonzero data_len, why doesn't
mount(2) just return EINVAL?

Giving file systems the responsibility for basic sanity checks on
syscall arguments strikes me as error-prone.


Re: CVS commit: src

2014-04-21 Thread Maxime Villard
Le 21/04/2014 16:29, Taylor R Campbell a écrit :
 
Date: Mon, 21 Apr 2014 08:20:22 +0200
From: Maxime Villard m...@m00nbsd.net
 
Le 21/04/2014 01:46, Taylor R Campbell a écrit :
 In most cases of the changes you made, there is already a test for the
 length of the data buffer.  Is this not guaranteed to be zero if data
 is null?  It seems to me that the length test ought to suffice, and if
 anything the null pointer test should be an assertion, not a check.
 
Not at all. 'data' and 'data_len' come from userpace. A user can set data
to NULL and data_len to a value high enough to bypass the data_len check.
 
 If a user passes in null data and nonzero data_len, why doesn't
 mount(2) just return EINVAL?

Yes it should; and data_len should not be modified by the kernel.

We also talked about that, and agreed on the fact that it was more a design
issue than a security issue.

 
 Giving file systems the responsibility for basic sanity checks on
 syscall arguments strikes me as error-prone.
 

Some fs's don't care at all about data and data_len. You can pass
data=NULL data_len=1024 to kernfs for example.

We don't want to change the behavior for NetBSD-6; it will be fixed in
-current.


Re: CVS commit: src/sys/dev/pcmcia

2014-04-21 Thread Joerg Sonnenberger
On Mon, Apr 21, 2014 at 08:24:21PM +, Paul Goyette wrote:
 Module Name:  src
 Committed By: pgoyette
 Date: Mon Apr 21 20:24:21 UTC 2014
 
 Modified Files:
   src/sys/dev/pcmcia: if_malo_pcmcia.c
 
 Log Message:
 Insert { ... } to keep gcc happy when DPRINTF() macro is empty.

This is wrong. DPRINTF should expand to do {} while (0) in !DEBUG case.

Joerg


Re: CVS commit: src/sys/dev/pcmcia

2014-04-21 Thread Paul Goyette

Ah, OK.  I will fix shortly.


On Tue, 22 Apr 2014, Joerg Sonnenberger wrote:


On Mon, Apr 21, 2014 at 08:24:21PM +, Paul Goyette wrote:

Module Name:src
Committed By:   pgoyette
Date:   Mon Apr 21 20:24:21 UTC 2014

Modified Files:
src/sys/dev/pcmcia: if_malo_pcmcia.c

Log Message:
Insert { ... } to keep gcc happy when DPRINTF() macro is empty.


This is wrong. DPRINTF should expand to do {} while (0) in !DEBUG case.

Joerg

!DSPAM:53559bfb212621094619121!




-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |  | pgoyette at netbsd.org  |
-