* bug#25895: Remove update-game-score
@ 2017-02-28 6:54 Glenn Morris
2017-03-09 8:50 ` Paul Eggert
0 siblings, 1 reply; 12+ messages in thread
From: Glenn Morris @ 2017-02-28 6:54 UTC (permalink / raw)
To: 25895
Package: emacs
Severity: wishlist
Version: 25.2
I'd like to suggest removing the update-game-score executable and the
associated machinery (Makefile rules, related gamegrid.el complexity).
update-game-score is a standalone executable whose job is to write a
system-wide score file for snake and tetris. For this purpose, it needs
to be installed setgid (or setuid) to the games group (or user), and the
central score files need to be pre-created with the relevant ownership
(see lib-src/Makefile).
In practice, I think this facility is very little used, and so should be
removed so that there are fewer things that need to be maintained.
Non-root users compiling and installing their own Emacs normally cannot
set the required permissions on the binary, or write to a central shared
score directory.
Distributions don't like having setuid/setgid binaries in their binary
packages because of the potential security implications, so tend to
strip them out. For example, in both the Red Hat rpm and Debian dpkg
packages for Emacs, update-game-score is not installed setgid or setuid.
So it doesn't do anything useful for these two major distributions and
their derivatives.
Ref eg
https://koji.fedoraproject.org/koji/fileinfo?rpmID=8691568&filename=/usr/libexec/emacs/25.1/x86_64-redhat-linux-gnu/update-game-score
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-02-28 6:54 bug#25895: Remove update-game-score Glenn Morris
@ 2017-03-09 8:50 ` Paul Eggert
2017-03-09 22:56 ` Glenn Morris
2017-03-10 6:42 ` Ulrich Mueller
0 siblings, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2017-03-09 8:50 UTC (permalink / raw)
To: Glenn Morris; +Cc: Ulrich Müller, 25895
[-- Attachment #1: Type: text/plain, Size: 304 bytes --]
Thanks, good suggestion. Proposed patch attached. I'll CC: this to Ulrich Müller
to see whether he has thoughts on this that are relevant to Gentoo, since he
sent in a Gentoo-related bug report about setgid a couple of years ago. Ulrich,
the new bug report is here:
https://bugs.gnu.org/25895
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-update-game-score.patch --]
[-- Type: text/x-diff; name="0001-Remove-update-game-score.patch", Size: 33661 bytes --]
From 666b4d38a007b5bb0067ca9bb7cc379423b0a4f9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 9 Mar 2017 00:40:01 -0800
Subject: [PATCH] Remove update-game-score
This program is very little used, and most distributions
do not use it properly due to setuid/setgid complications.
It is not worth the maintenance and security hassle (Bug#25895).
* .gitattributes: Remove lib-src/update-game-score.exe.manifest.
* .gitignore: Remove lib-src/update-game-score.
* Makefile.in (gamedir): Remove.
(epaths-force): Do not update PATH_GAME.
(uninstall): Do not remove snake-scores and tetris-scores.
* configure.ac: Remove --with-gameuser option.
(gamedir, gameuser, gamegroup, UPDATE_MANIFEST): Remove.
All uses removed.
* lib-src/Makefile.in (UPDATE_MANIFEST, gamedir, gameuser)
(gamegroup): Remove.
(UTILITIES): Remove update-game-score.
(SCRIPTS): Remove UPDATE_MANIFEST>
($(DESTDIR)${archlibdir}): Do not install gamedir programs and data.
(update-game-score${EXEEXT}): Remove.
* lib-src/update-game-score.c, lib-src/update-game-score.exe.manifest:
Remove.
* lisp/play/gamegrid.el (gamegrid-shared-game-dir)
(gamegrid-add-score-with-update-game-score-1):
* lisp/subr.el (shared-game-score-directory):
Make obsolete.
* lisp/play/gamegrid.el (gamegrid-add-score-with-update-game-score):
Remove use of auxiliary program.
* make-dist: Do not distribute update-game-score.exe.manifest.
* src/callproc.c (init_callproc):
No need to set Vshared_game_score_directory.
(syms_of_callproc) [!DOS_NT]:
Just set Vshared_game_score_directory to nil.
* src/epaths.in (PATH_GAME): Remove.
---
.gitattributes | 1 -
.gitignore | 1 -
Makefile.in | 11 +-
configure.ac | 25 +-
lib-src/Makefile.in | 40 +--
lib-src/update-game-score.c | 507 ---------------------------------
lib-src/update-game-score.exe.manifest | 10 -
lisp/play/gamegrid.el | 124 ++------
lisp/subr.el | 1 +
make-dist | 1 -
src/callproc.c | 12 -
src/epaths.in | 4 -
12 files changed, 24 insertions(+), 713 deletions(-)
delete mode 100644 lib-src/update-game-score.c
delete mode 100644 lib-src/update-game-score.exe.manifest
diff --git a/.gitattributes b/.gitattributes
index 22cea3d..59cc2ed 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -24,7 +24,6 @@ admin/charsets/mapfiles/PTCP154 whitespace=cr-at-eol
leim/MISC-DIC/cangjie-table.b5 whitespace=cr-at-eol
leim/MISC-DIC/cangjie-table.cns whitespace=cr-at-eol
leim/MISC-DIC/pinyin.map whitespace=cr-at-eol
-lib-src/update-game-score.exe.manifest whitespace=cr-at-eol
nt/nmake.defs whitespace=cr-at-eol
test/etags/c-src/dostorture.c whitespace=cr-at-eol
test/etags/cp-src/c.C whitespace=cr-at-eol
diff --git a/.gitignore b/.gitignore
index e8eb4fd..b54feb2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -186,7 +186,6 @@ lib-src/make-docfile
lib-src/movemail
lib-src/profile
lib-src/test-distrib
-lib-src/update-game-score
nextstep/Cocoa/Emacs.base/Contents/Info.plist
nextstep/Cocoa/Emacs.base/Contents/Resources/English.lproj
nextstep/Emacs.app/
diff --git a/Makefile.in b/Makefile.in
index 82fb91f..e0f1ff0 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -266,9 +266,6 @@ archlibdir=
# Where to put the etc/DOC file.
etcdocdir=@etcdocdir@
-# Where to install Emacs game score files.
-gamedir=@gamedir@
-
# ==================== Utility Programs for the Build ====================
# Allow the user to specify the install program.
@@ -347,8 +344,7 @@ epaths-force:
exit 1;; \
esac; \
done
- @(gamedir='${gamedir}'; \
- sed < ${srcdir}/src/epaths.in > epaths.h.$$$$ \
+ @(sed < ${srcdir}/src/epaths.in > epaths.h.$$$$ \
-e 's;\(#.*PATH_LOADSEARCH\).*$$;\1 "${standardlisppath}";' \
-e 's;\(#.*PATH_SITELOADSEARCH\).*$$;\1 "${locallisppath}";' \
-e 's;\(#.*PATH_DUMPLOADSEARCH\).*$$;\1 "${buildlisppath}";' \
@@ -359,7 +355,6 @@ epaths-force:
-e 's;\(#.*PATH_DATA\).*$$;\1 "${etcdir}";' \
-e 's;\(#.*PATH_BITMAPS\).*$$;\1 "${bitmapdir}";' \
-e 's;\(#.*PATH_X_DEFAULTS\).*$$;\1 "${x_default_search_path}";' \
- -e 's;\(#.*PATH_GAME\).*$$;\1 "${gamedir}";' \
-e 's;\(#.*PATH_DOC\).*$$;\1 "${etcdocdir}";') && \
${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
@@ -811,10 +806,6 @@ uninstall:
-rm -f "$(DESTDIR)${desktopdir}/${EMACS_NAME}.desktop"
-rm -f "$(DESTDIR)${appdatadir}/${EMACS_NAME}.appdata.xml"
-rm -f "$(DESTDIR)$(systemdunitdir)/${EMACS_NAME}.service"
- for file in snake-scores tetris-scores; do \
- file="$(DESTDIR)${gamedir}/$${file}"; \
- [ -s "$${file}" ] || rm -f "$$file"; \
- done
### Windows-specific uninstall target for removing programs produced
### in nt/, and its Posix do-nothing shadow.
diff --git a/configure.ac b/configure.ac
index ba944e6c..9a0c3c9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -203,7 +203,6 @@ AC_DEFUN
etcdir='${datadir}/emacs/${version}/etc'
archlibdir='${libexecdir}/emacs/${version}/${configuration}'
etcdocdir='${datadir}/emacs/${version}/etc'
-gamedir='${localstatedir}/games/emacs'
dnl Special option to disable the most of other options.
AC_ARG_WITH(all,
@@ -396,22 +395,6 @@ AC_DEFUN
[don't compress some files (.el, .info, etc.) when installing. Equivalent to:
make GZIP_PROG= install])
-AC_ARG_WITH(gameuser,dnl
-[AS_HELP_STRING([--with-gameuser=USER_OR_GROUP],
- [user for shared game score files.
- An argument prefixed by ':' specifies a group instead.])])
-gameuser=
-gamegroup=
-# We don't test if we can actually chown/chgrp here, because configure
-# may run without root privileges. lib-src/Makefile.in will handle
-# any errors due to missing user/group gracefully.
-case ${with_gameuser} in
- no) ;;
- "" | yes) gamegroup=games ;;
- :*) gamegroup=${with_gameuser#:} ;;
- *) gameuser=${with_gameuser} ;;
-esac
-
AC_ARG_WITH([gnustep-conf],dnl
[AS_HELP_STRING([--with-gnustep-conf=FILENAME],
[name of GNUstep configuration file to use on systems where the command
@@ -1966,7 +1949,6 @@ AC_DEFUN
CLIENTW=
W32_RES_LINK=
EMACS_MANIFEST=
-UPDATE_MANIFEST=
if test "${with_w32}" != no; then
case "${opsys}" in
cygwin)
@@ -2034,7 +2016,6 @@ AC_DEFUN
# the rc file), not a linker script.
W32_RES_LINK="-Wl,emacs.res"
else
- UPDATE_MANIFEST=update-game-score.exe.manifest
W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o"
W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -lusp10"
@@ -2054,7 +2035,6 @@ AC_DEFUN
AC_SUBST(W32_LIBS)
AC_SUBST(EMACSRES)
AC_SUBST(EMACS_MANIFEST)
-AC_SUBST(UPDATE_MANIFEST)
AC_SUBST(CLIENTRES)
AC_SUBST(CLIENTW)
AC_SUBST(W32_RES_LINK)
@@ -4892,9 +4872,6 @@ AC_DEFUN
AC_SUBST(archlibdir)
AC_SUBST(etcdocdir)
AC_SUBST(bitmapdir)
-AC_SUBST(gamedir)
-AC_SUBST(gameuser)
-AC_SUBST(gamegroup)
## FIXME? Nothing uses @LD_SWITCH_X_SITE@.
## src/Makefile.in did add LD_SWITCH_X_SITE (as a cpp define) to the
## end of LIBX_BASE, but nothing ever set it.
@@ -5432,7 +5409,7 @@ AC_DEFUN
dnl You might wonder (I did) why epaths.h is generated by running make,
dnl rather than just letting configure generate it from epaths.in.
dnl One reason is that the various paths are not fully expanded (see above);
-dnl eg gamedir=${prefix}/var/games/emacs.
+dnl e.g., etcdocdir='${datadir}/emacs/${version}/etc'.
dnl Secondly, the GNU Coding standards require that one should be able
dnl to run 'make prefix=/some/where/else' and override the values set
dnl by configure. This also explains the 'move-if-change' test and
diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index 88f6280..d31e87a 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -40,7 +40,6 @@ C_SWITCH_MACHINE=
PROFILING_CFLAGS = @PROFILING_CFLAGS@
WARN_CFLAGS = @WARN_CFLAGS@
WERROR_CFLAGS = @WERROR_CFLAGS@
-UPDATE_MANIFEST = @UPDATE_MANIFEST@
# Program name transformation.
TRANSFORM = @program_transform_name@
@@ -130,10 +129,6 @@ abs_top_srcdir=
# to '../configure'.
archlibdir=@archlibdir@
-gamedir=@gamedir@
-gameuser=@gameuser@
-gamegroup=@gamegroup@
-
# ==================== Utility Programs for the Build =================
# ../configure figures out the correct values for these.
@@ -155,14 +150,13 @@ INSTALLABLES =
# Things that Emacs runs internally, or during the build process,
# which should not be installed in bindir.
-UTILITIES = profile${EXEEXT} movemail${EXEEXT} hexl${EXEEXT} \
- update-game-score${EXEEXT}
+UTILITIES = profile${EXEEXT} movemail${EXEEXT} hexl${EXEEXT}
DONT_INSTALL= make-docfile${EXEEXT}
# Like UTILITIES, but they're not system-dependent, and should not be
# deleted by the distclean target.
-SCRIPTS= rcs2log $(UPDATE_MANIFEST)
+SCRIPTS= rcs2log
# All files that are created by the linker, i.e., whose names end in ${EXEEXT}.
EXE_FILES = ${INSTALLABLES} ${UTILITIES} ${DONT_INSTALL}
@@ -258,9 +252,6 @@ maybe-blessmail:
## Install the internal utilities. Until they are installed, we can
## just run them directly from lib-src.
-## If the chown/chmod commands fail, that is not a big deal.
-## update-game-score will detect at runtime that it is not setuid,
-## and handle things accordingly.
$(DESTDIR)${archlibdir}: all
@echo
@echo "Installing utilities run internally by Emacs."
@@ -272,28 +263,6 @@ $(DESTDIR)${archlibdir}:
"$(DESTDIR)${archlibdir}/$$file" || exit; \
done ; \
fi
- umask 022 && ${MKDIR_P} "$(DESTDIR)${gamedir}" && \
- touch "$(DESTDIR)${gamedir}/snake-scores" \
- "$(DESTDIR)${gamedir}/tetris-scores"
-ifneq ($(gameuser),)
- if chown ${gameuser} \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" && \
- chmod u+s,go-r \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"; \
- then \
- chown ${gameuser} "$(DESTDIR)${gamedir}" && \
- chmod u=rwx,g=rx,o=rx "$(DESTDIR)${gamedir}"; \
- fi
-else ifneq ($(gamegroup),)
- if chgrp ${gamegroup} \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" && \
- chmod g+s,o-r \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"; \
- then \
- chgrp ${gamegroup} "$(DESTDIR)${gamedir}" && \
- chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}"; \
- fi
-endif
exp_archlibdir=`cd "$(DESTDIR)${archlibdir}" && /bin/pwd` && \
if [ "$$exp_archlibdir" != "`cd ${srcdir} && /bin/pwd`" ]; then \
for file in ${SCRIPTS}; do \
@@ -415,11 +384,6 @@ ntlib.o:
hexl${EXEEXT}: ${srcdir}/hexl.c $(NTLIB) $(config_h)
$(AM_V_CCLD)$(CC) ${ALL_CFLAGS} $< $(LOADLIBES) -o $@
-update-game-score${EXEEXT}: ${srcdir}/update-game-score.c $(NTLIB) $(config_h)
- $(AM_V_CCLD)$(CC) ${ALL_CFLAGS} \
- -DHAVE_SHARED_GAME_DIR="\"$(gamedir)\"" \
- $< $(NTLIB) $(LOADLIBES) -o $@
-
emacsclient.res: ../nt/emacsclient.rc $(NTINC)/../icons/emacs.ico
$(AM_V_RC)$(WINDRES) -O coff --include-dir=$(NTINC)/.. -o $@ $<
diff --git a/lib-src/update-game-score.c b/lib-src/update-game-score.c
deleted file mode 100644
index 5edc8e7..0000000
--- a/lib-src/update-game-score.c
+++ /dev/null
@@ -1,507 +0,0 @@
-/* update-game-score.c --- Update a score file
-
-Copyright (C) 2002-2017 Free Software Foundation, Inc.
-
-Author: Colin Walters <walters@debian.org>
-
-This file is part of GNU Emacs.
-
-GNU Emacs is free software: you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation, either version 3 of the License, or (at
-your option) any later version.
-
-GNU Emacs is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
-
-
-/* This program allows a game to securely and atomically update a
- score file. It should be installed either setuid or setgid, owned
- by an appropriate user or group like `games'.
-
- Alternatively, it can be compiled without HAVE_SHARED_GAME_DIR
- defined, and in that case it will store scores in the user's home
- directory (it should NOT be setuid).
-
- Created 2002/03/22.
-*/
-
-#include <config.h>
-
-#include <unistd.h>
-#include <errno.h>
-#include <inttypes.h>
-#include <limits.h>
-#include <string.h>
-#include <stdlib.h>
-#include <stdio.h>
-#include <time.h>
-#include <pwd.h>
-#include <ctype.h>
-#include <fcntl.h>
-#include <sys/stat.h>
-#include <getopt.h>
-
-#ifdef WINDOWSNT
-#include "ntlib.h"
-#endif
-
-#ifndef min
-# define min(a,b) ((a) < (b) ? (a) : (b))
-#endif
-
-#define MAX_ATTEMPTS 5
-#define MAX_DATA_LEN 1024
-
-static _Noreturn void
-usage (int err)
-{
- fprintf (stdout, "Usage: update-game-score [-m MAX] [-r] [-d DIR] game/scorefile SCORE DATA\n");
- fprintf (stdout, " update-game-score -h\n");
- fprintf (stdout, " -h\t\tDisplay this help.\n");
- fprintf (stdout, " -m MAX\t\tLimit the maximum number of scores to MAX.\n");
- fprintf (stdout, " -r\t\tSort the scores in increasing order.\n");
- fprintf (stdout, " -d DIR\t\tStore scores in DIR (only if not setuid).\n");
- exit (err);
-}
-
-static int lock_file (const char *filename, void **state);
-static int unlock_file (const char *filename, void *state);
-
-struct score_entry
-{
- char *score;
- char *user_data;
-};
-
-#define MAX_SCORES min (PTRDIFF_MAX, SIZE_MAX / sizeof (struct score_entry))
-
-static int read_scores (const char *filename, struct score_entry **scores,
- ptrdiff_t *count, ptrdiff_t *alloc);
-static int push_score (struct score_entry **scores, ptrdiff_t *count,
- ptrdiff_t *size, struct score_entry const *newscore);
-static void sort_scores (struct score_entry *scores, ptrdiff_t count,
- bool reverse);
-static int write_scores (const char *filename, mode_t mode,
- const struct score_entry *scores, ptrdiff_t count);
-
-static _Noreturn void
-lose (const char *msg)
-{
- fprintf (stderr, "%s\n", msg);
- exit (EXIT_FAILURE);
-}
-
-static _Noreturn void
-lose_syserr (const char *msg)
-{
- fprintf (stderr, "%s: %s\n", msg,
- errno ? strerror (errno) : "Invalid data in score file");
- exit (EXIT_FAILURE);
-}
-
-static char *
-get_user_id (void)
-{
- struct passwd *buf = getpwuid (getuid ());
- if (!buf || strchr (buf->pw_name, ' ') || strchr (buf->pw_name, '\n'))
- {
- intmax_t uid = getuid ();
- char *name = malloc (sizeof uid * CHAR_BIT / 3 + 4);
- if (name)
- sprintf (name, "%"PRIdMAX, uid);
- return name;
- }
- return buf->pw_name;
-}
-
-static const char *
-get_prefix (bool privileged, const char *user_prefix)
-{
- if (privileged)
- {
-#ifdef HAVE_SHARED_GAME_DIR
- return HAVE_SHARED_GAME_DIR;
-#else
- lose ("This program was compiled without HAVE_SHARED_GAME_DIR,\n"
- "and should not run with elevated privileges.");
-#endif
- }
- if (user_prefix == NULL)
- lose ("Not using a shared game directory, and no prefix given.");
- return user_prefix;
-}
-
-static char *
-normalize_integer (char *num)
-{
- bool neg;
- char *p;
- while (*num != '\n' && isspace (*num))
- num++;
- neg = *num == '-';
- num += neg || *num == '-';
-
- if (*num == '0')
- {
- while (*++num == '0')
- continue;
- neg &= !!*num;
- num -= !*num;
- }
-
- for (p = num; '0' <= *p && *p <= '9'; p++)
- continue;
-
- if (*p || p == num)
- {
- errno = 0;
- return 0;
- }
-
- if (neg)
- *--num = '-';
- return num;
-}
-
-int
-main (int argc, char **argv)
-{
- int c;
- bool running_suid, running_sgid;
- void *lockstate;
- char *scorefile;
- char *end, *nl, *user, *data;
- const char *prefix, *user_prefix = NULL;
- struct score_entry *scores;
- struct score_entry newscore;
- bool reverse = false;
- ptrdiff_t scorecount, scorealloc;
- ptrdiff_t max_scores = MAX_SCORES;
-
- srand (time (0));
-
- while ((c = getopt (argc, argv, "hrm:d:")) != -1)
- switch (c)
- {
- case 'h':
- usage (EXIT_SUCCESS);
- break;
- case 'd':
- user_prefix = optarg;
- break;
- case 'r':
- reverse = 1;
- break;
- case 'm':
- {
- intmax_t m = strtoimax (optarg, &end, 10);
- if (optarg == end || *end || m < 0)
- usage (EXIT_FAILURE);
- max_scores = min (m, MAX_SCORES);
- }
- break;
- default:
- usage (EXIT_FAILURE);
- }
-
- if (argc - optind != 3)
- usage (EXIT_FAILURE);
-
- running_suid = (getuid () != geteuid ());
- running_sgid = (getgid () != getegid ());
- if (running_suid && running_sgid)
- lose ("This program can run either suid or sgid, but not both.");
-
- prefix = get_prefix (running_suid || running_sgid, user_prefix);
-
- scorefile = malloc (strlen (prefix) + strlen (argv[optind]) + 2);
- if (!scorefile)
- lose_syserr ("Couldn't allocate score file");
-
- char *z = stpcpy (scorefile, prefix);
- *z++ = '/';
- strcpy (z, argv[optind]);
-
- newscore.score = normalize_integer (argv[optind + 1]);
- if (! newscore.score)
- {
- fprintf (stderr, "%s: Invalid score\n", argv[optind + 1]);
- return EXIT_FAILURE;
- }
-
- user = get_user_id ();
- if (! user)
- lose_syserr ("Couldn't determine user id");
- data = argv[optind + 2];
- if (strlen (data) > MAX_DATA_LEN)
- data[MAX_DATA_LEN] = '\0';
- nl = strchr (data, '\n');
- if (nl)
- *nl = '\0';
- newscore.user_data = malloc (strlen (user) + 1 + strlen (data) + 1);
- if (! newscore.user_data
- || sprintf (newscore.user_data, "%s %s", user, data) < 0)
- lose_syserr ("Memory exhausted");
-
- if (lock_file (scorefile, &lockstate) < 0)
- lose_syserr ("Failed to lock scores file");
-
- if (read_scores (scorefile, &scores, &scorecount, &scorealloc) < 0)
- {
- unlock_file (scorefile, lockstate);
- lose_syserr ("Failed to read scores file");
- }
- if (push_score (&scores, &scorecount, &scorealloc, &newscore) < 0)
- {
- unlock_file (scorefile, lockstate);
- lose_syserr ("Failed to add score");
- }
- sort_scores (scores, scorecount, reverse);
- /* Limit the number of scores. If we're using reverse sorting, then
- also increment the beginning of the array, to skip over the
- *smallest* scores. Otherwise, just decrementing the number of
- scores suffices, since the smallest is at the end. */
- if (scorecount > max_scores)
- {
- if (reverse)
- scores += scorecount - max_scores;
- scorecount = max_scores;
- }
- if (write_scores (scorefile, running_sgid ? 0664 : 0644,
- scores, scorecount) < 0)
- {
- unlock_file (scorefile, lockstate);
- lose_syserr ("Failed to write scores file");
- }
- if (unlock_file (scorefile, lockstate) < 0)
- lose_syserr ("Failed to unlock scores file");
- exit (EXIT_SUCCESS);
-}
-
-static char *
-read_score (char *p, struct score_entry *score)
-{
- score->score = p;
- p = strchr (p, ' ');
- if (!p)
- return p;
- *p++ = 0;
- score->user_data = p;
- p = strchr (p, '\n');
- if (!p)
- return p;
- *p++ = 0;
- return p;
-}
-
-static int
-read_scores (const char *filename, struct score_entry **scores,
- ptrdiff_t *count, ptrdiff_t *alloc)
-{
- char *p, *filedata;
- ptrdiff_t filesize, nread;
- struct stat st;
- FILE *f = fopen (filename, "r");
- if (!f)
- return -1;
- if (fstat (fileno (f), &st) != 0)
- return -1;
- if (! (0 <= st.st_size && st.st_size < min (PTRDIFF_MAX, SIZE_MAX)))
- {
- errno = EOVERFLOW;
- return -1;
- }
- filesize = st.st_size;
- filedata = malloc (filesize + 1);
- if (! filedata)
- return -1;
- nread = fread (filedata, 1, filesize + 1, f);
- if (filesize < nread)
- {
- errno = 0;
- return -1;
- }
- if (nread < filesize)
- filesize = nread;
- if (ferror (f) || fclose (f) != 0)
- return -1;
- filedata[filesize] = 0;
- if (strlen (filedata) != filesize)
- {
- errno = 0;
- return -1;
- }
-
- *scores = 0;
- *count = *alloc = 0;
- for (p = filedata; p < filedata + filesize; )
- {
- struct score_entry entry;
- p = read_score (p, &entry);
- if (!p)
- {
- errno = 0;
- return -1;
- }
- if (push_score (scores, count, alloc, &entry) < 0)
- return -1;
- }
- return 0;
-}
-
-static int
-score_compare (const void *a, const void *b)
-{
- const struct score_entry *sa = (const struct score_entry *) a;
- const struct score_entry *sb = (const struct score_entry *) b;
- char *sca = sa->score;
- char *scb = sb->score;
- size_t lena, lenb;
- bool nega = *sca == '-';
- bool negb = *scb == '-';
- int diff = nega - negb;
- if (diff)
- return diff;
- if (nega)
- {
- char *tmp = sca;
- sca = scb + 1;
- scb = tmp + 1;
- }
- lena = strlen (sca);
- lenb = strlen (scb);
- if (lena != lenb)
- return lenb < lena ? -1 : 1;
- return strcmp (scb, sca);
-}
-
-static int
-score_compare_reverse (const void *a, const void *b)
-{
- return score_compare (b, a);
-}
-
-int
-push_score (struct score_entry **scores, ptrdiff_t *count, ptrdiff_t *size,
- struct score_entry const *newscore)
-{
- struct score_entry *newscores = *scores;
- if (*count == *size)
- {
- ptrdiff_t newsize = *size;
- if (newsize <= 0)
- newsize = 1;
- else if (newsize <= MAX_SCORES / 2)
- newsize *= 2;
- else if (newsize < MAX_SCORES)
- newsize = MAX_SCORES;
- else
- {
- errno = ENOMEM;
- return -1;
- }
- newscores = realloc (newscores, sizeof *newscores * newsize);
- if (!newscores)
- return -1;
- *scores = newscores;
- *size = newsize;
- }
- newscores[*count] = *newscore;
- (*count) += 1;
- return 0;
-}
-
-static void
-sort_scores (struct score_entry *scores, ptrdiff_t count, bool reverse)
-{
- qsort (scores, count, sizeof *scores,
- reverse ? score_compare_reverse : score_compare);
-}
-
-static int
-write_scores (const char *filename, mode_t mode,
- const struct score_entry *scores, ptrdiff_t count)
-{
- int fd;
- FILE *f;
- ptrdiff_t i;
- char *tempfile = malloc (strlen (filename) + strlen (".tempXXXXXX") + 1);
- if (!tempfile)
- return -1;
- strcpy (stpcpy (tempfile, filename), ".tempXXXXXX");
- fd = mkostemp (tempfile, 0);
- if (fd < 0)
- return -1;
-#ifndef DOS_NT
- if (fchmod (fd, mode) != 0)
- return -1;
-#endif
- f = fdopen (fd, "w");
- if (! f)
- return -1;
- for (i = 0; i < count; i++)
- if (fprintf (f, "%s %s\n", scores[i].score, scores[i].user_data) < 0)
- return -1;
- if (fclose (f) != 0)
- return -1;
- if (rename (tempfile, filename) != 0)
- return -1;
- return 0;
-}
-
-static int
-lock_file (const char *filename, void **state)
-{
- int fd;
- struct stat buf;
- int attempts = 0;
- const char *lockext = ".lockfile";
- char *lockpath = malloc (strlen (filename) + strlen (lockext) + 60);
- if (!lockpath)
- return -1;
- strcpy (stpcpy (lockpath, filename), lockext);
- *state = lockpath;
-
- while ((fd = open (lockpath, O_CREAT | O_EXCL, 0600)) < 0)
- {
- if (errno != EEXIST)
- return -1;
- attempts++;
-
- /* Break the lock if it is over an hour old, or if we've tried
- more than MAX_ATTEMPTS times. We won't corrupt the file, but
- we might lose some scores. */
- if (MAX_ATTEMPTS < attempts
- || (stat (lockpath, &buf) == 0 && 60 * 60 < time (0) - buf.st_ctime))
- {
- if (unlink (lockpath) != 0 && errno != ENOENT)
- return -1;
- attempts = 0;
- }
-
- sleep ((rand () & 1) + 1);
- }
-
- close (fd);
- return 0;
-}
-
-static int
-unlock_file (const char *filename, void *state)
-{
- char *lockpath = (char *) state;
- int saved_errno = errno;
- int ret = unlink (lockpath);
- int unlink_errno = errno;
- free (lockpath);
- errno = ret < 0 ? unlink_errno : saved_errno;
- return ret;
-}
-
-/* update-game-score.c ends here */
diff --git a/lib-src/update-game-score.exe.manifest b/lib-src/update-game-score.exe.manifest
deleted file mode 100644
index 1db836b..0000000
--- a/lib-src/update-game-score.exe.manifest
+++ /dev/null
@@ -1,10 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
- <v3:trustInfo xmlns:v3="urn:schemas-microsoft-com:asm.v3">
- <v3:security>
- <v3:requestedPrivileges>
- <v3:requestedExecutionLevel level="asInvoker" />
- </v3:requestedPrivileges>
- </v3:security>
- </v3:trustInfo>
-</assembly>
diff --git a/lisp/play/gamegrid.el b/lisp/play/gamegrid.el
index b0ccbd3..4f3be71 100644
--- a/lisp/play/gamegrid.el
+++ b/lisp/play/gamegrid.el
@@ -433,17 +433,9 @@ gamegrid-kill-timer
(defun gamegrid-add-score (file score)
"Add the current score to the high score file.
-On POSIX systems there may be a shared game directory for all users in
-which the scorefiles are kept. On such systems Emacs doesn't create
-the score file FILE in this directory, if it doesn't already exist.
-In this case Emacs searches for FILE in the directory specified by
-`gamegrid-user-score-file-directory' and creates it there, if
-necessary.
-
-To add the score file for a game to the system wide shared game
-directory, create the file with the shell command \"touch\" in this
-directory and make sure that it is owned by the correct user and
-group. You probably need special user privileges to do this.
+On POSIX systems Emacs searches for FILE in the directory
+specified by `gamegrid-user-score-file-directory' and creates it
+there, if necessary.
On non-POSIX systems Emacs searches for FILE in the directory
specified by the variable `temporary-file-directory'. If necessary,
@@ -455,112 +447,34 @@ gamegrid-add-score
(gamegrid-add-score-with-update-game-score file score))))
-;; On POSIX systems there are four cases to distinguish:
+;; On POSIX systems there are two cases to distinguish:
;; 1. FILE is an absolute filename. Then it should be a file in
;; temporary file directory. This is the way,
;; `gamegrid-add-score' was supposed to be used in the past and
;; is covered here for backward-compatibility.
;;
-;; 2. The helper program "update-game-score" is setgid or setuid
-;; and the file FILE does already exist in a system wide shared
-;; game directory. This should be the normal case on POSIX
-;; systems, if the game was installed system wide. Use
-;; "update-game-score" to add the score to the file in the
-;; shared game directory.
-;;
-;; 3. "update-game-score" is setgid/setuid, but the file FILE does
-;; *not* exist in the system wide shared game directory. Use
-;; `gamegrid-add-score-insecure' to create--if necessary--and
-;; update FILE. This is for the case that a user has installed
-;; a game on her own.
-;;
-;; 4. "update-game-score" is not setgid/setuid. Use it to
-;; create/update FILE in the user's home directory. There is
-;; presumably no shared game directory.
+;; 2. Create/update FILE in the user's home directory.
(defvar gamegrid-shared-game-dir)
+(make-obsolete-variable 'gamegrid-shared-game-dir nil "26.1")
(defun gamegrid-add-score-with-update-game-score (file score)
- (let ((gamegrid-shared-game-dir
- (not (zerop (logand (file-modes
- (expand-file-name "update-game-score"
- exec-directory))
- #o6000)))))
- (cond ((file-name-absolute-p file)
- (gamegrid-add-score-insecure file score))
- ((and gamegrid-shared-game-dir
- (file-exists-p (expand-file-name file shared-game-score-directory)))
- ;; Use the setgid (or setuid) "update-game-score" program
- ;; to update a system-wide score file.
- (gamegrid-add-score-with-update-game-score-1 file
- (expand-file-name file shared-game-score-directory) score))
- ;; Else: Add the score to a score file in the user's home
- ;; directory.
- (gamegrid-shared-game-dir
- ;; If `gamegrid-shared-game-dir' is non-nil, then
- ;; "update-gamescore" program is setuid, so don't use it.
- (unless (file-exists-p
- (directory-file-name gamegrid-user-score-file-directory))
- (make-directory gamegrid-user-score-file-directory t))
- (gamegrid-add-score-insecure file score
- gamegrid-user-score-file-directory))
- (t
- (unless (file-exists-p
- (directory-file-name gamegrid-user-score-file-directory))
- (make-directory gamegrid-user-score-file-directory t))
- (let ((f (expand-file-name file
- gamegrid-user-score-file-directory)))
- (unless (file-exists-p f)
- (write-region "" nil f nil 'silent nil 'excl))
- (gamegrid-add-score-with-update-game-score-1 file f score))))))
+ (if (file-name-absolute-p file)
+ (gamegrid-add-score-insecure file score)
+ ;; Add the score to a score file in the user's home directory.
+ (unless (file-exists-p
+ (directory-file-name gamegrid-user-score-file-directory))
+ (make-directory gamegrid-user-score-file-directory t))
+ (let ((f (expand-file-name file gamegrid-user-score-file-directory)))
+ (unless (file-exists-p f)
+ (write-region "" nil f nil 'silent nil 'excl))
+ (gamegrid-add-score-insecure f score))))
(defun gamegrid-add-score-with-update-game-score-1 (file target score)
- (let ((default-directory "/")
- (errbuf (generate-new-buffer " *update-game-score loss*"))
- (marker-string (concat
- (user-full-name)
- " <"
- (cond ((fboundp 'user-mail-address)
- (user-mail-address))
- ((boundp 'user-mail-address)
- user-mail-address)
- (t ""))
- "> "
- (current-time-string))))
- ;; This can be called from a timer, so enable local quits.
- (with-local-quit
- (apply
- 'call-process
- (append
- (list
- (expand-file-name "update-game-score" exec-directory)
- nil errbuf nil
- "-m" (int-to-string gamegrid-score-file-length)
- "-d" (if gamegrid-shared-game-dir
- (expand-file-name shared-game-score-directory)
- (file-name-directory target))
- file
- (int-to-string score)
- marker-string))))
- (if (buffer-modified-p errbuf)
- (progn
- (display-buffer errbuf)
- (error "Failed to update game score file"))
- (kill-buffer errbuf))
- (let ((buf (find-buffer-visiting target)))
- (save-excursion
- (if buf
- (progn
- (switch-to-buffer buf)
- (revert-buffer nil t nil)
- (display-buffer buf))
- (find-file-read-only target))
- (goto-char (point-min))
- (search-forward (concat (int-to-string score)
- " " (user-login-name) " "
- marker-string) nil t)
- (beginning-of-line)))))
+ (gamegrid-add-score-with-update-game-score target score))
+(make-obsolete 'gamegrid-add-score-with-update-game-score-1
+ 'gamegrid-add-score-with-update-game-score "26.1")
(defun gamegrid-add-score-insecure (file score &optional directory)
(save-excursion
diff --git a/lisp/subr.el b/lisp/subr.el
index 6b04038..f735556 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1454,6 +1454,7 @@ 'unfocus-frame
(make-obsolete-variable 'command-debug-status
"expect it to be removed in a future version." "25.2")
+(make-obsolete-variable 'shared-game-score-directory nil "26.1")
;; Lisp manual only updated in 22.1.
(define-obsolete-variable-alias 'executing-macro 'executing-kbd-macro
diff --git a/make-dist b/make-dist
index 41203b2..e85a2d6 100755
--- a/make-dist
+++ b/make-dist
@@ -459,7 +459,6 @@ files=
ln [a-zA-Z]*.[ch] ../${tempdir}/lib-src
ln ChangeLog.*[0-9] Makefile.in README ../${tempdir}/lib-src
ln rcs2log ../${tempdir}/lib-src
- ln update-game-score.exe.manifest ../${tempdir}/lib-src)
echo "Making links to 'm4'"
(cd m4
diff --git a/src/callproc.c b/src/callproc.c
index 08fa6e9..98369ba 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1583,14 +1583,6 @@ init_callproc (void)
sh = getenv ("SHELL");
Vshell_file_name = build_string (sh ? sh : "/bin/sh");
-
-#ifdef DOS_NT
- Vshared_game_score_directory = Qnil;
-#else
- Vshared_game_score_directory = build_unibyte_string (PATH_GAME);
- if (NILP (Ffile_accessible_directory_p (Vshared_game_score_directory)))
- Vshared_game_score_directory = Qnil;
-#endif
}
void
@@ -1661,11 +1653,7 @@ includes this. */);
DEFVAR_LISP ("shared-game-score-directory", Vshared_game_score_directory,
doc: /* Directory of score files for games which come with GNU Emacs.
If this variable is nil, then Emacs is unable to use a shared directory. */);
-#ifdef DOS_NT
Vshared_game_score_directory = Qnil;
-#else
- Vshared_game_score_directory = build_string (PATH_GAME);
-#endif
DEFVAR_LISP ("initial-environment", Vinitial_environment,
doc: /* List of environment variables inherited from the parent process.
diff --git a/src/epaths.in b/src/epaths.in
index c491d3b..fd61029 100644
--- a/src/epaths.in
+++ b/src/epaths.in
@@ -70,9 +70,5 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
macro, and is then used to set the Info-default-directory-list. */
#define PATH_INFO "/usr/local/share/info"
-/* Where Emacs should store game score files. */
-#define PATH_GAME "/usr/local/var/games/emacs"
-
/* Where Emacs should look for the application default file. */
#define PATH_X_DEFAULTS "/usr/lib/X11/%L/%T/%N%C%S:/usr/lib/X11/%l/%T/%N%C%S:/usr/lib/X11/%T/%N%C%S:/usr/lib/X11/%L/%T/%N%S:/usr/lib/X11/%l/%T/%N%S:/usr/lib/X11/%T/%N%S"
-
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-09 8:50 ` Paul Eggert
@ 2017-03-09 22:56 ` Glenn Morris
2017-03-10 6:42 ` Ulrich Mueller
1 sibling, 0 replies; 12+ messages in thread
From: Glenn Morris @ 2017-03-09 22:56 UTC (permalink / raw)
To: Paul Eggert; +Cc: Ulrich Müller, 25895
Thanks for the (as always) extremely thorough patch.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-09 8:50 ` Paul Eggert
2017-03-09 22:56 ` Glenn Morris
@ 2017-03-10 6:42 ` Ulrich Mueller
2017-03-10 8:16 ` Paul Eggert
2017-03-10 22:40 ` Paul Eggert
1 sibling, 2 replies; 12+ messages in thread
From: Ulrich Mueller @ 2017-03-10 6:42 UTC (permalink / raw)
To: Paul Eggert; +Cc: 25895
>>>>> On Thu, 9 Mar 2017, Paul Eggert wrote:
> Thanks, good suggestion. Proposed patch attached. I'll CC: this to
> Ulrich Müller to see whether he has thoughts on this that are
> relevant to Gentoo, since he sent in a Gentoo-related bug report
> about setgid a couple of years ago. Ulrich, the new bug report is
> here:
> https://bugs.gnu.org/25895
I am not happy about this. Gentoo installs update-game-score as a
setgid binary and it is working well. I agree that setuid binaries are
generally frowned upon, but that is much less the case for setgid.
In fact, installing such binaries setgid and beloging to a "games" or
similar group is valid policy in both Debian and Gentoo:
https://www.debian.org/doc/debian-policy/ch-customized-programs.html#s11.11
https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Policies#Games
$ ls -l /usr/libexec/emacs/25.2/x86_64-pc-linux-gnu/
total 88
-rwxr-xr-x 1 root root 10344 Mar 2 09:35 hexl
-rwxr-xr-x 1 root root 27104 Mar 2 09:35 movemail
-rwxr-xr-x 1 root root 6184 Mar 2 09:35 profile
-rwxr-xr-x 1 root root 21154 Mar 2 09:35 rcs2log
-rwxr-s--x 1 root gamestat 14656 Mar 2 09:35 update-game-score
Also I don't buy the argument that these files were a maintenance
burden. Browsing the git history of lib-src/update-game-score.c and
lisp/play/gamegrid.el, I see that the last nontrivial change to them
was a patch that I submitted myself more than two years ago.
I think you need better reasons for removal of working functionality.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-10 6:42 ` Ulrich Mueller
@ 2017-03-10 8:16 ` Paul Eggert
2017-03-10 22:40 ` Paul Eggert
1 sibling, 0 replies; 12+ messages in thread
From: Paul Eggert @ 2017-03-10 8:16 UTC (permalink / raw)
To: Ulrich Mueller; +Cc: 25895
Ulrich Mueller wrote:
> I don't buy the argument that these files were a maintenance
> burden.
It's not simply my own burden (which is not always reflected by commit log
entries, and where even the "trivial" changes are more work for me). It's all
the people who have to review this code for all the distributions. Even making
something setgid is a big deal. Obviously the Debian and Fedora people don't
want to bother.
If people were really using this program to support multiuser games, it'd be
worth the trouble. But I don't have the sense that the feature is actually used.
That being said, the simplest thing is to do nothing.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-10 6:42 ` Ulrich Mueller
2017-03-10 8:16 ` Paul Eggert
@ 2017-03-10 22:40 ` Paul Eggert
2017-03-11 6:32 ` Eli Zaretskii
2017-03-11 8:30 ` Ulrich Mueller
1 sibling, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2017-03-10 22:40 UTC (permalink / raw)
To: Ulrich Mueller; +Cc: 25895
[-- Attachment #1: Type: text/plain, Size: 440 bytes --]
On second thought, how about if we install update-game-score only when
the builder specifies the game user or group. This will let Gentoo roll
along much as before, while simplifying installation for distributions
that don't support or bother to configure a game user or group. Although
this doesn't simplify the Emacs source as much as the previously
proposed patch, it's still better than what we have now. Proposed patch
attached.
[-- Attachment #2: 0001-Install-update-game-score-only-on-request.patch --]
[-- Type: application/x-patch, Size: 14437 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-10 22:40 ` Paul Eggert
@ 2017-03-11 6:32 ` Eli Zaretskii
2017-03-12 8:45 ` Paul Eggert
2017-03-11 8:30 ` Ulrich Mueller
1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-03-11 6:32 UTC (permalink / raw)
To: Paul Eggert; +Cc: ulm, 25895
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 10 Mar 2017 14:40:39 -0800
> Cc: 25895@debbugs.gnu.org
>
> On second thought, how about if we install update-game-score only when
> the builder specifies the game user or group. This will let Gentoo roll
> along much as before, while simplifying installation for distributions
> that don't support or bother to configure a game user or group. Although
> this doesn't simplify the Emacs source as much as the previously
> proposed patch, it's still better than what we have now. Proposed patch
> attached.
Thanks.
Please add comments to the affected Makefile.in files to explain the
conditions related to user/group.
I also don't understand why you unconditionally removed this program
from the Windows builds and installations: the problem with setgid
doesn't exist on Windows, so nothing should prevent Windows
installations from having this program, right?
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-10 22:40 ` Paul Eggert
2017-03-11 6:32 ` Eli Zaretskii
@ 2017-03-11 8:30 ` Ulrich Mueller
1 sibling, 0 replies; 12+ messages in thread
From: Ulrich Mueller @ 2017-03-11 8:30 UTC (permalink / raw)
To: Paul Eggert; +Cc: 25895
>>>>> On Fri, 10 Mar 2017, Paul Eggert wrote:
> On second thought, how about if we install update-game-score only
> when the builder specifies the game user or group. This will let
> Gentoo roll along much as before, while simplifying installation for
> distributions that don't support or bother to configure a game user
> or group. Although this doesn't simplify the Emacs source as much as
> the previously proposed patch, it's still better than what we have
> now.
I like this idea much better than the first one.
> Proposed patch attached.
Tested and it doesn't work. With the --with-gameuser=":gamestat"
configure option, it properly installs the update-game-score binary.
In spite of this, Emacs doesn't use the shared directory, but creates
a score file in the user's home directory instead.
The reason is that gameuser and gamegroup are not propagated to the
top-level Makefile:
--- emacs/Makefile.in~
+++ emacs/Makefile.in
@@ -268,6 +268,8 @@
# Where to install Emacs game score files.
gamedir=@gamedir@
+gameuser=@gameuser@
+gamegroup=@gamegroup@
# ==================== Utility Programs for the Build ====================
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-11 6:32 ` Eli Zaretskii
@ 2017-03-12 8:45 ` Paul Eggert
2017-03-12 13:53 ` Ulrich Mueller
2017-03-12 15:14 ` Eli Zaretskii
0 siblings, 2 replies; 12+ messages in thread
From: Paul Eggert @ 2017-03-12 8:45 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: ulm, 25895
[-- Attachment #1: Type: text/plain, Size: 787 bytes --]
Eli Zaretskii wrote:
> Please add comments to the affected Makefile.in files to explain the
> conditions related to user/group.
Done in the attached revised patch, which also fixes the bug Ulrich Müller noted.
> I also don't understand why you unconditionally removed this program
> from the Windows builds and installations: the problem with setgid
> doesn't exist on Windows, so nothing should prevent Windows
> installations from having this program, right?
It's more the other way round. On platforms without setuid/setgid, Emacs can use
its already-existing code to update the score file itself. The auxiliary program
is needed only on platforms that have setuid/setgid, to avoid the security
problems that would ensue if we made Emacs itself setuid/setgid.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Install-update-game-score-only-on-request.patch --]
[-- Type: text/x-diff; name="0001-Install-update-game-score-only-on-request.patch", Size: 15367 bytes --]
From 659d605d6456779be0c6703eceda508b76e751f0 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 12 Mar 2017 00:17:42 -0800
Subject: [PATCH] Install update-game-score only on request
Most distributions do not install update-game-score properly
due to setuid/setgid complications, so install it only when
the installer specifies a user or group (Bug#25895).
* .gitattributes: Remove lib-src/update-game-score.exe.manifest.
* Makefile.in (gameuser, gamegroup, use_gamedir, PATH_GAME):
New vars.
(epaths-force): Use PATH_GAME.
(uninstall): Remove snake-scores and tetris-scores only if shared.
* configure.ac: Default --with-gameuser to 'no'.
(UPDATE_MANIFEST): Remove.
* etc/NEWS: Mention this.
* lib-src/Makefile.in (UPDATE_MANIFEST): Remove.
(use_gamedir): New macro.
(UTILITIES): Remove update-game-score unless use_gamedir.
(SCRIPTS): Remove $(UPDATE_MANIFEST).
($(DESTDIR)${archlibdir}): Install game directory program and data
only if use_gamedir.
* lib-src/update-game-score.exe.manifest: Remove, as
update-game-score is no longer installed on MS-Windows.
* lisp/play/gamegrid.el (gamegrid-add-score-with-update-game-score):
Use auxiliary program only if setuid or setgid.
* make-dist: Do not distribute update-game-score.exe.manifest.
* src/callproc.c (init_callproc):
Set Vshared_game_score_directory based on PATH_GAME, not DOS_NT.
(syms_of_callproc): Remove unnecessary initialization of
Vshared_game_score_directory.
---
.gitattributes | 1 -
Makefile.in | 13 ++++++++--
configure.ac | 12 +++------
etc/NEWS | 5 ++++
lib-src/Makefile.in | 45 ++++++++++++++--------------------
lib-src/update-game-score.exe.manifest | 10 --------
lisp/play/gamegrid.el | 29 ++++++++--------------
make-dist | 1 -
src/callproc.c | 20 ++++++---------
9 files changed, 56 insertions(+), 80 deletions(-)
delete mode 100644 lib-src/update-game-score.exe.manifest
diff --git a/.gitattributes b/.gitattributes
index 22cea3d..59cc2ed 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -24,7 +24,6 @@ admin/charsets/mapfiles/PTCP154 whitespace=cr-at-eol
leim/MISC-DIC/cangjie-table.b5 whitespace=cr-at-eol
leim/MISC-DIC/cangjie-table.cns whitespace=cr-at-eol
leim/MISC-DIC/pinyin.map whitespace=cr-at-eol
-lib-src/update-game-score.exe.manifest whitespace=cr-at-eol
nt/nmake.defs whitespace=cr-at-eol
test/etags/c-src/dostorture.c whitespace=cr-at-eol
test/etags/cp-src/c.C whitespace=cr-at-eol
diff --git a/Makefile.in b/Makefile.in
index 82fb91f..bf673f4 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -266,8 +266,12 @@ archlibdir=
# Where to put the etc/DOC file.
etcdocdir=@etcdocdir@
-# Where to install Emacs game score files.
+# Where to install game score files, if gameuser or gamegroup is nonempty.
gamedir=@gamedir@
+gameuser=@gameuser@
+gamegroup=@gamegroup@
+# Nonempty if and only if a shared gamedir is used.
+use_gamedir=$(gameuser)$(gamegroup)
# ==================== Utility Programs for the Build ====================
@@ -334,6 +338,9 @@ etc-emacsver:
${srcdir}/build-aux/move-if-change emacsver.tex.$$$$ \
${srcdir}/etc/refcards/emacsver.tex
+# The shared gamedir name as a C string literal, or a null ptr if not in use.
+PATH_GAME = $(if $(use_gamedir),"$(gamedir)",((char const *) 0))
+
# Generate epaths.h from epaths.in. This target is invoked by 'configure'.
# See comments in configure.ac for why it is done this way, as opposed
# to just letting configure generate epaths.h from epaths.in in a
@@ -359,7 +366,7 @@ epaths-force:
-e 's;\(#.*PATH_DATA\).*$$;\1 "${etcdir}";' \
-e 's;\(#.*PATH_BITMAPS\).*$$;\1 "${bitmapdir}";' \
-e 's;\(#.*PATH_X_DEFAULTS\).*$$;\1 "${x_default_search_path}";' \
- -e 's;\(#.*PATH_GAME\).*$$;\1 "${gamedir}";' \
+ -e 's;\(#.*PATH_GAME\).*$$;\1 $(PATH_GAME);' \
-e 's;\(#.*PATH_DOC\).*$$;\1 "${etcdocdir}";') && \
${srcdir}/build-aux/move-if-change epaths.h.$$$$ src/epaths.h
@@ -811,10 +818,12 @@ uninstall:
-rm -f "$(DESTDIR)${desktopdir}/${EMACS_NAME}.desktop"
-rm -f "$(DESTDIR)${appdatadir}/${EMACS_NAME}.appdata.xml"
-rm -f "$(DESTDIR)$(systemdunitdir)/${EMACS_NAME}.service"
+ ifneq (,$(use_gamedir))
for file in snake-scores tetris-scores; do \
file="$(DESTDIR)${gamedir}/$${file}"; \
[ -s "$${file}" ] || rm -f "$$file"; \
done
+ endif
### Windows-specific uninstall target for removing programs produced
### in nt/, and its Posix do-nothing shadow.
diff --git a/configure.ac b/configure.ac
index ba944e6c..4d9ba96 100644
--- a/configure.ac
+++ b/configure.ac
@@ -402,12 +402,9 @@ AC_DEFUN
An argument prefixed by ':' specifies a group instead.])])
gameuser=
gamegroup=
-# We don't test if we can actually chown/chgrp here, because configure
-# may run without root privileges. lib-src/Makefile.in will handle
-# any errors due to missing user/group gracefully.
case ${with_gameuser} in
- no) ;;
- "" | yes) gamegroup=games ;;
+ '' | no) ;;
+ yes) gamegroup=games ;;
:*) gamegroup=${with_gameuser#:} ;;
*) gameuser=${with_gameuser} ;;
esac
@@ -1966,7 +1963,6 @@ AC_DEFUN
CLIENTW=
W32_RES_LINK=
EMACS_MANIFEST=
-UPDATE_MANIFEST=
if test "${with_w32}" != no; then
case "${opsys}" in
cygwin)
@@ -2034,7 +2030,6 @@ AC_DEFUN
# the rc file), not a linker script.
W32_RES_LINK="-Wl,emacs.res"
else
- UPDATE_MANIFEST=update-game-score.exe.manifest
W32_OBJ="$W32_OBJ w32.o w32console.o w32heap.o w32inevt.o w32proc.o"
W32_LIBS="$W32_LIBS -lwinmm -lgdi32 -lcomdlg32"
W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -lusp10"
@@ -2054,7 +2049,6 @@ AC_DEFUN
AC_SUBST(W32_LIBS)
AC_SUBST(EMACSRES)
AC_SUBST(EMACS_MANIFEST)
-AC_SUBST(UPDATE_MANIFEST)
AC_SUBST(CLIENTRES)
AC_SUBST(CLIENTW)
AC_SUBST(W32_RES_LINK)
@@ -5432,7 +5426,7 @@ AC_DEFUN
dnl You might wonder (I did) why epaths.h is generated by running make,
dnl rather than just letting configure generate it from epaths.in.
dnl One reason is that the various paths are not fully expanded (see above);
-dnl eg gamedir=${prefix}/var/games/emacs.
+dnl e.g., gamedir='${localstatedir}/games/emacs'.
dnl Secondly, the GNU Coding standards require that one should be able
dnl to run 'make prefix=/some/where/else' and override the values set
dnl by configure. This also explains the 'move-if-change' test and
diff --git a/etc/NEWS b/etc/NEWS
index f0df0a7..cd829bf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -65,6 +65,11 @@ emacs-version and erc-cmd-SV functions, and the leave the following
variables nil: emacs-build-system, emacs-build-time,
erc-emacs-build-time.
+** The configure option '--with-gameuser' now defaults to 'no',
+as this appears to be the most common configuration in practice.
+When it is 'no', the shared game directory and the auxiliary program
+update-game-score are no longer needed and are not installed.
+
** Emacs no longer works on IRIX. We expect that Emacs users are not
affected by this, as SGI stopped supporting IRIX in December 2013.
diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index 88f6280..608b978 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -40,7 +40,6 @@ C_SWITCH_MACHINE=
PROFILING_CFLAGS = @PROFILING_CFLAGS@
WARN_CFLAGS = @WARN_CFLAGS@
WERROR_CFLAGS = @WERROR_CFLAGS@
-UPDATE_MANIFEST = @UPDATE_MANIFEST@
# Program name transformation.
TRANSFORM = @program_transform_name@
@@ -130,9 +129,12 @@ abs_top_srcdir=
# to '../configure'.
archlibdir=@archlibdir@
+# Where to install game score files, if gameuser or gamegroup is nonempty.
gamedir=@gamedir@
gameuser=@gameuser@
gamegroup=@gamegroup@
+# Nonempty if and only if a shared gamedir is used.
+use_gamedir=$(gameuser)$(gamegroup)
# ==================== Utility Programs for the Build =================
@@ -156,13 +158,13 @@ INSTALLABLES =
# Things that Emacs runs internally, or during the build process,
# which should not be installed in bindir.
UTILITIES = profile${EXEEXT} movemail${EXEEXT} hexl${EXEEXT} \
- update-game-score${EXEEXT}
+ $(and $(use_gamedir), update-game-score${EXEEXT})
DONT_INSTALL= make-docfile${EXEEXT}
# Like UTILITIES, but they're not system-dependent, and should not be
# deleted by the distclean target.
-SCRIPTS= rcs2log $(UPDATE_MANIFEST)
+SCRIPTS= rcs2log
# All files that are created by the linker, i.e., whose names end in ${EXEEXT}.
EXE_FILES = ${INSTALLABLES} ${UTILITIES} ${DONT_INSTALL}
@@ -258,9 +260,6 @@ maybe-blessmail:
## Install the internal utilities. Until they are installed, we can
## just run them directly from lib-src.
-## If the chown/chmod commands fail, that is not a big deal.
-## update-game-score will detect at runtime that it is not setuid,
-## and handle things accordingly.
$(DESTDIR)${archlibdir}: all
@echo
@echo "Installing utilities run internally by Emacs."
@@ -272,28 +271,22 @@ $(DESTDIR)${archlibdir}:
"$(DESTDIR)${archlibdir}/$$file" || exit; \
done ; \
fi
- umask 022 && ${MKDIR_P} "$(DESTDIR)${gamedir}" && \
+ ifneq (,$(use_gamedir))
+ umask 022 && ${MKDIR_P} "$(DESTDIR)${gamedir}"
touch "$(DESTDIR)${gamedir}/snake-scores" \
"$(DESTDIR)${gamedir}/tetris-scores"
-ifneq ($(gameuser),)
- if chown ${gameuser} \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" && \
- chmod u+s,go-r \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"; \
- then \
- chown ${gameuser} "$(DESTDIR)${gamedir}" && \
- chmod u=rwx,g=rx,o=rx "$(DESTDIR)${gamedir}"; \
- fi
-else ifneq ($(gamegroup),)
- if chgrp ${gamegroup} \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}" && \
- chmod g+s,o-r \
- "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"; \
- then \
- chgrp ${gamegroup} "$(DESTDIR)${gamedir}" && \
- chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}"; \
- fi
-endif
+ ifneq (,$(gameuser))
+ chown ${gameuser} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
+ chmod u+s,go-r "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
+ chown ${gameuser} "$(DESTDIR)${gamedir}"
+ chmod u=rwx,g=rx,o=rx "$(DESTDIR)${gamedir}"
+ else
+ chgrp ${gamegroup} "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
+ chmod g+s,o-r "$(DESTDIR)${archlibdir}/update-game-score${EXEEXT}"
+ chgrp ${gamegroup} "$(DESTDIR)${gamedir}"
+ chmod u=rwx,g=rwx,o=rx "$(DESTDIR)${gamedir}"
+ endif
+ endif
exp_archlibdir=`cd "$(DESTDIR)${archlibdir}" && /bin/pwd` && \
if [ "$$exp_archlibdir" != "`cd ${srcdir} && /bin/pwd`" ]; then \
for file in ${SCRIPTS}; do \
diff --git a/lib-src/update-game-score.exe.manifest b/lib-src/update-game-score.exe.manifest
deleted file mode 100644
index 1db836b..0000000
--- a/lib-src/update-game-score.exe.manifest
+++ /dev/null
@@ -1,10 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
-<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
- <v3:trustInfo xmlns:v3="urn:schemas-microsoft-com:asm.v3">
- <v3:security>
- <v3:requestedPrivileges>
- <v3:requestedExecutionLevel level="asInvoker" />
- </v3:requestedPrivileges>
- </v3:security>
- </v3:trustInfo>
-</assembly>
diff --git a/lisp/play/gamegrid.el b/lisp/play/gamegrid.el
index b0ccbd3..0386a89 100644
--- a/lisp/play/gamegrid.el
+++ b/lisp/play/gamegrid.el
@@ -475,17 +475,19 @@ gamegrid-add-score
;; update FILE. This is for the case that a user has installed
;; a game on her own.
;;
-;; 4. "update-game-score" is not setgid/setuid. Use it to
-;; create/update FILE in the user's home directory. There is
-;; presumably no shared game directory.
+;; 4. "update-game-score" does not exist or is not setgid/setuid.
+;; Create/update FILE in the user's home directory, without
+;; using "update-game-score". There is presumably no shared
+;; game directory.
(defvar gamegrid-shared-game-dir)
(defun gamegrid-add-score-with-update-game-score (file score)
(let ((gamegrid-shared-game-dir
- (not (zerop (logand (file-modes
- (expand-file-name "update-game-score"
- exec-directory))
+ (not (zerop (logand (or (file-modes
+ (expand-file-name "update-game-score"
+ exec-directory))
+ 0)
#o6000)))))
(cond ((file-name-absolute-p file)
(gamegrid-add-score-insecure file score))
@@ -497,23 +499,12 @@ gamegrid-add-score-with-update-game-score
(expand-file-name file shared-game-score-directory) score))
;; Else: Add the score to a score file in the user's home
;; directory.
- (gamegrid-shared-game-dir
- ;; If `gamegrid-shared-game-dir' is non-nil, then
- ;; "update-gamescore" program is setuid, so don't use it.
- (unless (file-exists-p
- (directory-file-name gamegrid-user-score-file-directory))
- (make-directory gamegrid-user-score-file-directory t))
- (gamegrid-add-score-insecure file score
- gamegrid-user-score-file-directory))
(t
(unless (file-exists-p
(directory-file-name gamegrid-user-score-file-directory))
(make-directory gamegrid-user-score-file-directory t))
- (let ((f (expand-file-name file
- gamegrid-user-score-file-directory)))
- (unless (file-exists-p f)
- (write-region "" nil f nil 'silent nil 'excl))
- (gamegrid-add-score-with-update-game-score-1 file f score))))))
+ (gamegrid-add-score-insecure file score
+ gamegrid-user-score-file-directory)))))
(defun gamegrid-add-score-with-update-game-score-1 (file target score)
(let ((default-directory "/")
diff --git a/make-dist b/make-dist
index 41203b2..e85a2d6 100755
--- a/make-dist
+++ b/make-dist
@@ -459,7 +459,6 @@ files=
ln [a-zA-Z]*.[ch] ../${tempdir}/lib-src
ln ChangeLog.*[0-9] Makefile.in README ../${tempdir}/lib-src
ln rcs2log ../${tempdir}/lib-src
- ln update-game-score.exe.manifest ../${tempdir}/lib-src)
echo "Making links to 'm4'"
(cd m4
diff --git a/src/callproc.c b/src/callproc.c
index 08fa6e9..0504857 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1584,13 +1584,14 @@ init_callproc (void)
sh = getenv ("SHELL");
Vshell_file_name = build_string (sh ? sh : "/bin/sh");
-#ifdef DOS_NT
- Vshared_game_score_directory = Qnil;
-#else
- Vshared_game_score_directory = build_unibyte_string (PATH_GAME);
- if (NILP (Ffile_accessible_directory_p (Vshared_game_score_directory)))
- Vshared_game_score_directory = Qnil;
-#endif
+ Lisp_Object gamedir = Qnil;
+ if (PATH_GAME)
+ {
+ Lisp_Object path_game = build_unibyte_string (PATH_GAME);
+ if (file_accessible_directory_p (path_game))
+ gamedir = path_game;
+ }
+ Vshared_game_score_directory = gamedir;
}
void
@@ -1661,11 +1662,6 @@ includes this. */);
DEFVAR_LISP ("shared-game-score-directory", Vshared_game_score_directory,
doc: /* Directory of score files for games which come with GNU Emacs.
If this variable is nil, then Emacs is unable to use a shared directory. */);
-#ifdef DOS_NT
- Vshared_game_score_directory = Qnil;
-#else
- Vshared_game_score_directory = build_string (PATH_GAME);
-#endif
DEFVAR_LISP ("initial-environment", Vinitial_environment,
doc: /* List of environment variables inherited from the parent process.
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-12 8:45 ` Paul Eggert
@ 2017-03-12 13:53 ` Ulrich Mueller
2017-03-12 15:14 ` Eli Zaretskii
1 sibling, 0 replies; 12+ messages in thread
From: Ulrich Mueller @ 2017-03-12 13:53 UTC (permalink / raw)
To: Paul Eggert; +Cc: 25895
>>>>> On Sun, 12 Mar 2017, Paul Eggert wrote:
> Done in the attached revised patch, which also fixes the bug
> Ulrich Müller noted.
Ack.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-12 8:45 ` Paul Eggert
2017-03-12 13:53 ` Ulrich Mueller
@ 2017-03-12 15:14 ` Eli Zaretskii
2017-03-12 19:06 ` Paul Eggert
1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2017-03-12 15:14 UTC (permalink / raw)
To: Paul Eggert; +Cc: ulm, 25895
> Cc: ulm@gentoo.org, 25895@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 12 Mar 2017 00:45:13 -0800
>
> Eli Zaretskii wrote:
>
> > Please add comments to the affected Makefile.in files to explain the
> > conditions related to user/group.
>
> Done in the attached revised patch, which also fixes the bug Ulrich Müller noted.
Thanks, but I expected to see there the explanation of why we
sometimes expect user/group to be empty and sometimes not. IOW, I'd
like to have in the affected Makefile a comment which summarized the
rationale in the commit message, something akin to this:
> Most distributions do not install update-game-score properly
> due to setuid/setgid complications, so install it only when
> the installer specifies a user or group (Bug#25895).
> > I also don't understand why you unconditionally removed this program
> > from the Windows builds and installations: the problem with setgid
> > doesn't exist on Windows, so nothing should prevent Windows
> > installations from having this program, right?
>
> It's more the other way round. On platforms without setuid/setgid, Emacs can use
> its already-existing code to update the score file itself. The auxiliary program
> is needed only on platforms that have setuid/setgid, to avoid the security
> problems that would ensue if we made Emacs itself setuid/setgid.
Ah, yes. Thanks for clarifying this.
^ permalink raw reply [flat|nested] 12+ messages in thread
* bug#25895: Remove update-game-score
2017-03-12 15:14 ` Eli Zaretskii
@ 2017-03-12 19:06 ` Paul Eggert
0 siblings, 0 replies; 12+ messages in thread
From: Paul Eggert @ 2017-03-12 19:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: ulm, 25895-done
Eli Zaretskii wrote:
> I'd
> like to have in the affected Makefile a comment which summarized the
> rationale in the commit message
OK, I added a comment and installed the patch. Marking this bug as done.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-12 19:06 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-28 6:54 bug#25895: Remove update-game-score Glenn Morris
2017-03-09 8:50 ` Paul Eggert
2017-03-09 22:56 ` Glenn Morris
2017-03-10 6:42 ` Ulrich Mueller
2017-03-10 8:16 ` Paul Eggert
2017-03-10 22:40 ` Paul Eggert
2017-03-11 6:32 ` Eli Zaretskii
2017-03-12 8:45 ` Paul Eggert
2017-03-12 13:53 ` Ulrich Mueller
2017-03-12 15:14 ` Eli Zaretskii
2017-03-12 19:06 ` Paul Eggert
2017-03-11 8:30 ` Ulrich Mueller
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).