Re: CVS commit: src
Le 21/04/2014 01:46, Taylor R Campbell a écrit : > >From: "Maxime Villard" >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
Date: Mon, 21 Apr 2014 08:20:22 +0200 From: Maxime Villard 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
Le 21/04/2014 16:29, Taylor R Campbell a écrit : > >Date: Mon, 21 Apr 2014 08:20:22 +0200 >From: Maxime Villard > >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
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
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 |
-
