Date: Tue, 12 Apr 2005 08:05:51 -0700 (PDT) From: "ALeine" <aleine@austrosearch.net> To: bms@spc.org Cc: killing@multiplay.co.uk Subject: [patch] GBDE memory allocations bugs and mlockall(2) [Was Re: kernel killing processes when out of swap] Message-ID: <200504121505.j3CF5pDg020695@marlena.vvi.at>
next in thread | raw e-mail | index | archive | help
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 <sys/stat.h> #include <crypto/rijndael/rijndael.h> #include <crypto/sha2/sha2.h> +#include <sys/time.h> +#include <sys/resource.h> #include <sys/param.h> +#include <sys/mman.h> #include <sys/linker.h> #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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200504121505.j3CF5pDg020695>