Date: Mon, 17 Feb 2014 00:00:39 +0000 (UTC) From: Bryan Drewery <bdrewery@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r262006 - head/sys/kern Message-ID: <201402170000.s1H00dsP021119@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bdrewery Date: Mon Feb 17 00:00:39 2014 New Revision: 262006 URL: http://svnweb.freebsd.org/changeset/base/262006 Log: Fix M_FILEDESC leak in fdgrowtable() introduced in r244510. fdgrowtable() now only reallocates fd_map when necessary. This fixes fdgrowtable() to use the same logic as fdescfree() for when to free the fd_map. The logic in fdescfree() is intended to not free the initial static allocation, however the fd_map grows at a slower rate than the table does. The table is intended to hold 20 fd, but its initial map has many more slots than 20. The slot sizing causes NDSLOTS(20) through NDSLOTS(63) to be 1 which matches NDSLOTS(20), so fdescfree() was assuming that the fd_map was still the initial allocation and not freeing it. This partially reverts r244510 by reintroducing some of the logic it removed in fdgrowtable(). Reviewed by: mjg Approved by: bapt (mentor) MFC after: 2 weeks Modified: head/sys/kern/kern_descrip.c Modified: head/sys/kern/kern_descrip.c ============================================================================== --- head/sys/kern/kern_descrip.c Sun Feb 16 23:10:46 2014 (r262005) +++ head/sys/kern/kern_descrip.c Mon Feb 17 00:00:39 2014 (r262006) @@ -1521,7 +1521,7 @@ fdgrowtable(struct filedesc *fdp, int nf return; /* - * Allocate a new table and map. We need enough space for the + * Allocate a new table. We need enough space for the * file entries themselves and the struct freetable we will use * when we decommission the table and place it on the freelist. * We place the struct freetable in the middle so we don't have @@ -1529,16 +1529,20 @@ fdgrowtable(struct filedesc *fdp, int nf */ ntable = malloc(nnfiles * sizeof(ntable[0]) + sizeof(struct freetable), M_FILEDESC, M_ZERO | M_WAITOK); - nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC, - M_ZERO | M_WAITOK); - /* copy the old data over and point at the new tables */ memcpy(ntable, otable, onfiles * sizeof(*otable)); - memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap)); - - /* update the pointers and counters */ fdp->fd_ofiles = ntable; - fdp->fd_map = nmap; + + /* Allocate a new map only if the old is not large enough. It will + * grow at a slower rate than the table as it can map more + * entries than the table can hold. */ + if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) { + nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC, + M_ZERO | M_WAITOK); + /* copy over the old data and update the pointer */ + memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap)); + fdp->fd_map = nmap; + } /* * In order to have a valid pattern for fget_unlocked() @@ -1554,8 +1558,6 @@ fdgrowtable(struct filedesc *fdp, int nf * reference entries within it. Instead, place it on a freelist * which will be processed when the struct filedesc is released. * - * Do, however, free the old map. - * * Note that if onfiles == NDFILE, we're dealing with the original * static allocation contained within (struct filedesc0 *)fdp, * which must not be freed. @@ -1565,8 +1567,12 @@ fdgrowtable(struct filedesc *fdp, int nf fdp0 = (struct filedesc0 *)fdp; ft->ft_table = otable; SLIST_INSERT_HEAD(&fdp0->fd_free, ft, ft_next); - free(omap, M_FILEDESC); } + /* The map does not have the same possibility of threads still + * holding references to it. So always free it as long as it + * does not reference the original static allocation. */ + if (NDSLOTS(onfiles) > NDSLOTS(NDFILE)) + free(omap, M_FILEDESC); } /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201402170000.s1H00dsP021119>