Module Name: src
Committed By: pho
Date: Sun Jul 30 09:23:22 UTC 2023
Modified Files:
src/games/hack: hack.o_init.c
Log Message:
hack(6): Fix a segfault that occurs when ASLR is enabled
Prior to this change, savenames() would store "objects" in save files as a
blob, and restnames() would load it and overwrite "objects". But since
objclass::oc_name and oc_descr are pointers to string constants, they would
be invalid when the next time the process is spawned, and opening the
inventory would crash by dereferencing invalid pointers.
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.15 src/games/hack/hack.o_init.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/games/hack/hack.o_init.c
diff -u src/games/hack/hack.o_init.c:1.14 src/games/hack/hack.o_init.c:1.15
--- src/games/hack/hack.o_init.c:1.14 Sat Aug 6 20:42:43 2011
+++ src/games/hack/hack.o_init.c Sun Jul 30 09:23:21 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: hack.o_init.c,v 1.14 2011/08/06 20:42:43 dholland Exp $ */
+/* $NetBSD: hack.o_init.c,v 1.15 2023/07/30 09:23:21 pho Exp $ */
/*
* Copyright (c) 1985, Stichting Centrum voor Wiskunde en Informatica,
@@ -63,7 +63,7 @@
#include <sys/cdefs.h>
#ifndef lint
-__RCSID("$NetBSD: hack.o_init.c,v 1.14 2011/08/06 20:42:43 dholland Exp $");
+__RCSID("$NetBSD: hack.o_init.c,v 1.15 2023/07/30 09:23:21 pho Exp $");
#endif /* not lint */
#include <string.h>
@@ -184,15 +184,21 @@ savenames(int fd)
bwrite(fd, bases, sizeof bases);
bwrite(fd, objects, sizeof objects);
/*
- * as long as we use only one version of Hack/Quest we need not save
- * oc_name and oc_descr, but we must save oc_uname for all objects
+ * We must save not only oc_uname but also oc_name and oc_descr,
+ * because they are string constants whose pointer values aren't
+ * peristent when ASLR is enabled.
*/
for (i = 0; i < SIZE(objects); i++) {
- if (objects[i].oc_uname) {
- len = strlen(objects[i].oc_uname) + 1;
- bwrite(fd, &len, sizeof len);
- bwrite(fd, objects[i].oc_uname, len);
+#define SAVE_NAME_FIELD(FIELD) \
+ if (objects[i].FIELD) { \
+ len = strlen(objects[i].FIELD) + 1; \
+ bwrite(fd, &len, sizeof len); \
+ bwrite(fd, objects[i].FIELD, len); \
}
+ SAVE_NAME_FIELD(oc_name);
+ SAVE_NAME_FIELD(oc_descr);
+ SAVE_NAME_FIELD(oc_uname);
+#undef SAVE_NAME_FIELD
}
}
@@ -200,15 +206,25 @@ void
restnames(int fd)
{
int i;
- unsigned len;
+ size_t len;
mread(fd, bases, sizeof bases);
mread(fd, objects, sizeof objects);
- for (i = 0; i < SIZE(objects); i++)
- if (objects[i].oc_uname) {
- mread(fd, &len, sizeof len);
- objects[i].oc_uname = alloc(len);
- mread(fd, objects[i].oc_uname, len);
+ for (i = 0; i < SIZE(objects); i++) {
+#define RESTORE_NAME_FIELD(FIELD) \
+ if (objects[i].FIELD) { \
+ mread(fd, &len, sizeof len); \
+ objects[i].FIELD = alloc(len); \
+ mread(fd, __UNCONST(objects[i].FIELD), len); \
}
+ /*
+ * This leaks memory but who cares? Restoration only
+ * happens on the process startup.
+ */
+ RESTORE_NAME_FIELD(oc_name);
+ RESTORE_NAME_FIELD(oc_descr);
+ RESTORE_NAME_FIELD(oc_uname);
+#undef RESTORE_NAME_FIELD
+ }
}
int
