Re: CVS commit: src
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
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
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
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 | -