svn commit: r273552 - head/sys/kern

2014-10-23 Thread Xin LI 
Author: delphij
Date: Thu Oct 23 18:23:50 2014
New Revision: 273552
URL: https://svnweb.freebsd.org/changeset/base/273552

Log:
  Test if 'env' is NULL before doing memset() and strlen(),
  the caller may pass NULL to freeenv().

Modified:
  head/sys/kern/kern_environment.c

Modified: head/sys/kern/kern_environment.c
==
--- head/sys/kern/kern_environment.cThu Oct 23 18:03:27 2014
(r273551)
+++ head/sys/kern/kern_environment.cThu Oct 23 18:23:50 2014
(r273552)
@@ -262,7 +262,7 @@ void
 freeenv(char *env)
 {
 
-   if (dynamic_kenv) {
+   if (dynamic_kenv  env != NULL) {
memset(env, 0, strlen(env));
free(env, M_KENV);
}
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273552 - head/sys/kern

2014-10-23 Thread Dag-Erling Smørgrav 
Xin LI delp...@freebsd.org writes:
 Log:
   Test if 'env' is NULL before doing memset() and strlen(),
   the caller may pass NULL to freeenv().

If this is in response to a panic in early boot, the real bug is
elsewhere (see r273564).  Adding a NULL check here only hides it.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273552 - head/sys/kern

2014-10-23 Thread Xin Li 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 10/23/14 15:47, Dag-Erling Smørgrav wrote:
 Xin LI delp...@freebsd.org writes:
 Log: Test if 'env' is NULL before doing memset() and strlen(), 
 the caller may pass NULL to freeenv().
 
 If this is in response to a panic in early boot, the real bug is 
 elsewhere (see r273564).  Adding a NULL check here only hides it.

Yes that would fix it.  Does this look good to you?

Index: sys/kern/kern_environment.c
===
- --- sys/kern/kern_environment.c   (revision 273564)
+++ sys/kern/kern_environment.c (working copy)
@@ -262,7 +262,8 @@ void
 freeenv(char *env)
 {

- - if (dynamic_kenv  env != NULL) {
+   MPASS(env != NULL);
+   if (dynamic_kenv) {
memset(env, 0, strlen(env));
free(env, M_KENV);
}


Cheers,
- -- 
Xin LI delp...@delphij.nethttps://www.delphij.net/
FreeBSD - The Power to Serve!   Live free or die
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0

iQIcBAEBCgAGBQJUSYzoAAoJEJW2GBstM+nsSnYP/i4boJtEHPzOiJceF0x1OW+h
/Jn/ewoZh5itogPuGBcXB0QSNZIoQjH3x8Puv6S+bgfGwP6oXtWGpFHeH1EXtu0N
pUqaft3VP8d/8+QeM0l5GSrYWverDZM0FpwnhWC0L/+0Fen52pseLGJIYGmG0Z80
vBxiRokXmNtC467tgu5upSSXDftEu9RFsWPCtuTr2yP+RZYC98hdSTl2EufA0L3Z
ih46Iz+zIYcQt7ziryQ0nblGSuQy+AucqPa/0/fs9mSpqd0+fSphR+nd62P60yC4
mDUB093mMl/PhZY5IrfoU1DTxDdUJNoGc1wNdzKqsbxTbjpMJMeWr6dHX2pni8Ki
MCR+Hh6aMxC+P0g2n5cCmTTff8ghRXSW0/pSluJjyPALmYHXCUdnXWAj59hkbpUA
O6wOtth4rLi0ZcGPT4hNKu5bm+BvhQ4EEKjomB+WzZUmUL+/H8xoin2xEaItMs6T
uDaZqaknRACr5+TFuwVbBP77QOZ9i9EcqBIMOSkNw1E0NqdGLxTwud8IQCqMAONJ
ZHrwNOCM91sLk4ohjrPNzZzY3IXostBzRNNJQvFEXQJhbxJvQM3CDcr/QOoMajN2
1VWMbxrONgcyi8RTC8PvOgxRN36SFBJP0rxhrn465xGpAWsZ9M28sOWmKk2O4yhB
LHZuuMYanWSQ0+SYJrGT
=eSq9
-END PGP SIGNATURE-
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273552 - head/sys/kern

2014-10-23 Thread Mateusz Guzik 
On Thu, Oct 23, 2014 at 04:19:05PM -0700, Xin Li wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA512
 
 On 10/23/14 15:47, Dag-Erling Smørgrav wrote:
  Xin LI delp...@freebsd.org writes:
  Log: Test if 'env' is NULL before doing memset() and strlen(), 
  the caller may pass NULL to freeenv().
  
  If this is in response to a panic in early boot, the real bug is 
  elsewhere (see r273564).  Adding a NULL check here only hides it.
 
 Yes that would fix it.  Does this look good to you?
 
 Index: sys/kern/kern_environment.c
 ===
 - --- sys/kern/kern_environment.c (revision 273564)
 +++ sys/kern/kern_environment.c   (working copy)
 @@ -262,7 +262,8 @@ void
  freeenv(char *env)
  {
 
 - -   if (dynamic_kenv  env != NULL) {
 + MPASS(env != NULL);
 + if (dynamic_kenv) {
   memset(env, 0, strlen(env));
   free(env, M_KENV);
   }
 

There are at least 80 consumers of this function. Unless someone is up
to reviewing them all, can we go with a warning + backtrace for the time
being?

-- 
Mateusz Guzik mjguzik gmail.com
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org

Re: svn commit: r273552 - head/sys/kern

2014-10-23 Thread Bryan Drewery 
On 10/23/2014 6:21 PM, Mateusz Guzik wrote:
 On Thu, Oct 23, 2014 at 04:19:05PM -0700, Xin Li wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA512

 On 10/23/14 15:47, Dag-Erling Smørgrav wrote:
 Xin LI delp...@freebsd.org writes:
 Log: Test if 'env' is NULL before doing memset() and strlen(), 
 the caller may pass NULL to freeenv().

 If this is in response to a panic in early boot, the real bug is 
 elsewhere (see r273564).  Adding a NULL check here only hides it.

 Yes that would fix it.  Does this look good to you?

 Index: sys/kern/kern_environment.c
 ===
 - --- sys/kern/kern_environment.c(revision 273564)
 +++ sys/kern/kern_environment.c  (working copy)
 @@ -262,7 +262,8 @@ void
  freeenv(char *env)
  {

 - -  if (dynamic_kenv  env != NULL) {
 +MPASS(env != NULL);
 +if (dynamic_kenv) {
  memset(env, 0, strlen(env));
  free(env, M_KENV);
  }

 
 There are at least 80 consumers of this function. Unless someone is up
 to reviewing them all, can we go with a warning + backtrace for the time
 being?
 

My DEBUG_WARN could be used for that. I had not committed yet, but it is
ready.

-- 
Regards,
Bryan Drewery



signature.asc
Description: OpenPGP digital signature