From owner-freebsd-hackers@FreeBSD.ORG Tue Apr 12 15:05:30 2005 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3A81716A4CE for ; Tue, 12 Apr 2005 15:05:30 +0000 (GMT) Received: from marlena.vvi.at (marlena.vvi.at [208.252.225.59]) by mx1.FreeBSD.org (Postfix) with ESMTP id B2ED243D1D for ; Tue, 12 Apr 2005 15:05:29 +0000 (GMT) (envelope-from www@marlena.vvi.at) Received: from marlena.vvi.at (localhost.marlena.vvi.at [127.0.0.1]) by marlena.vvi.at (8.12.10/8.12.9) with ESMTP id j3CF5vh3020696; Tue, 12 Apr 2005 08:06:05 -0700 (PDT) (envelope-from www@marlena.vvi.at) Received: (from www@localhost) by marlena.vvi.at (8.12.10/8.12.10/Submit) id j3CF5pDg020695; Tue, 12 Apr 2005 08:05:51 -0700 (PDT) (envelope-from www) Date: Tue, 12 Apr 2005 08:05:51 -0700 (PDT) Message-Id: <200504121505.j3CF5pDg020695@marlena.vvi.at> Content-Type: multipart/mixed; boundary="----------=_1113318350-20694-0" Content-Transfer-Encoding: binary MIME-Version: 1.0 X-Mailer: MIME-tools 5.411 (Entity 5.404) From: "ALeine" To: bms@spc.org cc: freebsd-hackers@freebsd.org cc: killing@multiplay.co.uk Subject: [patch] GBDE memory allocations bugs and mlockall(2) [Was Re: kernel killing processes when out of swap] X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Apr 2005 15:05:30 -0000 This is a multi-part message in MIME format... ------------=_1113318350-20694-0 Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: binary bms@spc.org wrote: > It would be more appropriate if such processes use the mlockall(2) call > for the time being. Speaking of mlockall(2), almost two weeks ago when I posted here about CGD (and GBDE) memory allocation bugs I sent the attached patch for src/sbin/gbde/gbde.c to PHK, but since he never got back to me on that I would like to ask you guys to review the patch and commit it as appropriate, hopefully before the next release. The patch addresses memory allocation bugs where memory is used without checking first if allocation was successful. In one case this could lead to a segment violation with the core dump containing the sensitive info required to decrypt the GBDE volume. The patch also disables core dumping by using setrlimit(2)) and prevents paging by using mlockall(2). I have patched several similar memory allocation bugs in libgeom, so if anyone would like to see these bugs fixed let me know and I'll post that patch as well. Right now I get the feeling that due to memory overcommit in FreeBSD some people might think that these checks are not absolutely necessary since another larger process would be more likely to get killed before malloc(3) would return NULL, but I believe that one should not rely on that. ALeine ___________________________________________________________________ WebMail FREE http://mail.austrosearch.net ------------=_1113318350-20694-0 Content-Type: text/plain; name="gbde.c.20050330.patch" Content-Disposition: inline; filename="gbde.c.20050330.patch" Content-Transfer-Encoding: 8bit --- src/sbin/gbde/gbde.c.old Wed Mar 30 14:18:37 2005 +++ src/sbin/gbde/gbde.c Wed Mar 30 15:11:15 2005 @@ -85,7 +85,10 @@ #include #include #include +#include +#include #include +#include #include #define GBDEMOD "geom_bde" @@ -145,6 +148,19 @@ exit (1); } +static void +disable_core_dumping(void) +{ + struct rlimit rl; + int error; + + rl.rlim_cur = 0; + rl.rlim_max = 0; + error = setrlimit(RLIMIT_CORE, &rl); + if (error) + err(1, "setrlimit() failed to set core size to 0"); +} + void * g_read_data(struct g_consumer *cp, off_t offset, off_t length, int *error) { @@ -355,7 +371,10 @@ u_char *sbuf; off_t offset, offset2; + /* Allocate a sectorbuffer immediately and zero it */ sbuf = malloc(gl->sectorsize); + if (sbuf == NULL) + err(1, "malloc"); memset(sbuf, 0, gl->sectorsize); offset = (gl->lsector[key] & ~(gl->sectorsize - 1)); offset2 = lseek(dfd, offset, SEEK_SET); @@ -377,7 +396,10 @@ u_char *sbuf, *q; off_t offset, offset2; + /* Allocate a sectorbuffer */ sbuf = malloc(gl->sectorsize); + if (sbuf == NULL) + err(1, "malloc"); /* * Find the byte-offset in the lock sector where we will put the lock * data structure. We can put it any random place as long as the @@ -422,9 +444,7 @@ errx(1, "No -L option and no space in sector 0 for lockfile"); } - /* Allocate a sectorbuffer and fill it with random junk */ - if (sbuf == NULL) - err(1, "malloc"); + /* Fill sectorbuffer with random junk */ random_bits(sbuf, gl->sectorsize); /* Fill random bits in the spare field */ @@ -722,12 +742,17 @@ if (argc < 3) usage("Too few arguments\n"); - if ((i = modfind("g_bde")) < 0) { - /* need to load the gbde module */ - if (kldload(GBDEMOD) < 0 || modfind("g_bde") < 0) { - usage(GBDEMOD ": Kernel module not available\n"); - } - } + if ((i = modfind("g_bde")) < 0) { + /* need to load the gbde module */ + if (kldload(GBDEMOD) < 0 || modfind("g_bde") < 0) { + usage(GBDEMOD ": Kernel module not available\n"); + } + } + + disable_core_dumping(); + if (mlockall(MCL_FUTURE)) + err(1, "mlockall() failed to lock all memory"); + doopen = 0; if (!strcmp(argv[1], "attach")) { action = ACT_ATTACH; ------------=_1113318350-20694-0--