Bug#700421: wdm shouldn't use /dev/mem
Package: wdm
Version: 1.28-13
Severity: normal
Tags: upstream patch
Hi,
this is my first bug report against a debian package so I very well
might've missed something in the process - please excuse if so in
advance.
Here's the deal: wdm still uses /dev/mem in genauth.c to generate a
tmp key and it shouldn't. The kernel currently allows userspace to
read around 640K of /dev/mem for compatibility reasons with X, the
abovementioned one being one of them.
However, the modern way of getting random data is /dev/urandom and I've
attached a patch below which converts wdm to do that.
Patch is ontop of the master branch of
git://git.debian.org/collab-maint/wdm.git and fixes the issue.
Thanks.
This is a multi-part MIME message sent by reportbug.
--===1255078598==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Package: wdm
Version: 1.28-13
Severity: normal
Tags: upstream patch
-- System Information:
Debian Release: wheezy/sid
APT prefers testing
APT policy: (500, 'testing')
Architecture: i386 (i686)
Kernel: Linux 3.5.0+ (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL
set to en_US.UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages wdm depends on:
ii debconf [debconf-2.0] 1.5.40
ii libc6 2.13-16
ii libpam-modules 1.1.3-2
ii libpam-runtime 1.1.3-2
ii libpam0g 1.1.3-7.1
ii libselinux12.1.9-5
ii libwings2 0.95.3-2
ii libwraster30.95.3-2
ii libwutil2 0.95.3-2
ii libx11-6 2:1.5.0-1
ii libxau61:1.0.7-1
ii libxdmcp6 1:1.1.1-1
ii libxinerama1 2:1.1.2-1
ii libxmu62:1.1.1-1
ii psmisc 22.13-1
ii x11-apps 7.6+5
ii x11-common 1:7.6+7
ii x11-utils 7.6+3
ii x11-xserver-utils 7.6+3
wdm recommends no packages.
Versions of packages wdm suggests:
ii xfonts-base 1:1.0.3
-- Configuration Files:
/etc/X11/wdm/wdm-config [Errno 13] Permission denied: u'/etc/X11/wdm/wdm-config'
-- debconf information:
* shared/default-x-display-manager: wdm
wdm/daemon_name: /usr/bin/wdm
--===1255078598==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="08_do_not_use_dev_mem.patch"
Author: Borislav Petkov
Description: Do not use /dev/mem as a randomFile
diff --git a/debian/man/wdm.1x b/debian/man/wdm.1x
index 5f468d5fcc36..968acf293f2f 100644
--- a/debian/man/wdm.1x
+++ b/debian/man/wdm.1x
@@ -202,7 +202,7 @@ to pass on to the \fIXsetup\fP,
.IP \fBDisplayManager.randomFile\fP
A file to checksum to generate the seed of authorization keys.
This should be a file that changes frequently.
-The default is \fI/dev/mem\fP.
+The default is \fI/dev/urandom\fP.
.IP \fBDisplayManager.greeterLib\fP
On systems that support a dynamically-loadable greeter library, the
name of the library. The default is
diff --git a/doc/wdm.man.in b/doc/wdm.man.in
index da44f2860d6b..4c22f1db58e6 100644
--- a/doc/wdm.man.in
+++ b/doc/wdm.man.in
@@ -202,7 +202,7 @@ to pass on to the \fIXsetup\fP,
.IP \fBDisplayManager.randomFile\fP
A file to checksum to generate the seed of authorization keys.
This should be a file that changes frequently.
-The default is \fI/dev/mem\fP.
+The default is \fI/dev/urandom\fP.
.IP \fBDisplayManager.greeterLib\fP
On systems that support a dynamically-loadable greeter library, the
name of the library. The default is
diff --git a/src/wdm/genauth.c b/src/wdm/genauth.c
index e478d936be9d..3156adf8427a 100644
--- a/src/wdm/genauth.c
+++ b/src/wdm/genauth.c
@@ -71,37 +71,26 @@ longtochars (long l, unsigned char *c)
#if !defined(ARC4_RANDOM) && !defined(DEV_RANDOM)
static int
-sumFile (char *name, long sum[2])
+sumFile (char *name, long sum[], unsigned n)
{
-longbuf[1024*2];
intcnt;
intfd;
-intloops;
-intreads;
-inti;
-int ret_status = 0;
+int ret_status = 1;
fd = open (name, O_RDONLY);
if (fd < 0) {
WDMError("Cannot open randomFile \"%s\", errno = %d\n", name, errno);
return 0;
}
-#ifdef FRAGILE_DEV_MEM
-if (strcmp(name, "/dev/mem") == 0) lseek (fd, (off_t) 0x10, SEEK_SET);
-#endif
-reads = FILE_LIMIT;
-sum[0] = 0;
-sum[1] = 0;
-while ((cnt = read (fd, (char *) buf, sizeof (buf))) > 0 && --reads > 0) {
- loops = cnt / (2 * sizeof (long));
- for (i = 0; i < loops; i+= 2) {
- sum[0] += buf[i];
- sum[1] += buf[i+1];
- ret_status = 1;
- }
-}
-if (cnt < 0)
+
+memset(sum, 0, n);
+
+
Bug#700422: wdm shouldn't use /dev/mem
Package: wdm
Version: 1.28-13
Severity: normal
Tags: upstream patch
Hi,
this is my first reporting a bug against a debian package so I very well
might've missed something in the process.
Here's the deal: wdm still uses /dev/mem in genauth.c to generate a tmp
key and it shouldn't. The kernel currently allows userspace to read <
640K of /dev/mem for compatibility reasons with X. The modern way of
getting two random longs is /dev/urandom and I've a patch below which
converts wdm to do that.
Patch is ontop of the master branch of
git://git.debian.org/collab-maint/wdm.git and fixes the issue.
Thanks.
-- System Information:
Debian Release: wheezy/sid
APT prefers testing
APT policy: (500, 'testing')
Architecture: i386 (i686)
Kernel: Linux 3.5.0+ (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL
set to en_US.UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages wdm depends on:
ii debconf [debconf-2.0] 1.5.40
ii libc6 2.13-16
ii libpam-modules 1.1.3-2
ii libpam-runtime 1.1.3-2
ii libpam0g 1.1.3-7.1
ii libselinux12.1.9-5
ii libwings2 0.95.3-2
ii libwraster30.95.3-2
ii libwutil2 0.95.3-2
ii libx11-6 2:1.5.0-1
ii libxau61:1.0.7-1
ii libxdmcp6 1:1.1.1-1
ii libxinerama1 2:1.1.2-1
ii libxmu62:1.1.1-1
ii psmisc 22.13-1
ii x11-apps 7.6+5
ii x11-common 1:7.6+7
ii x11-utils 7.6+3
ii x11-xserver-utils 7.6+3
wdm recommends no packages.
Versions of packages wdm suggests:
ii xfonts-base 1:1.0.3
-- Configuration Files:
/etc/X11/wdm/wdm-config [Errno 13] Permission denied: u'/etc/X11/wdm/wdm-config'
-- debconf information:
* shared/default-x-display-manager: wdm
wdm/daemon_name: /usr/bin/wdm
diff --git a/debian/man/wdm.1x b/debian/man/wdm.1x
index 5f468d5fcc36..968acf293f2f 100644
--- a/debian/man/wdm.1x
+++ b/debian/man/wdm.1x
@@ -202,7 +202,7 @@ to pass on to the \fIXsetup\fP,
.IP \fBDisplayManager.randomFile\fP
A file to checksum to generate the seed of authorization keys.
This should be a file that changes frequently.
-The default is \fI/dev/mem\fP.
+The default is \fI/dev/urandom\fP.
.IP \fBDisplayManager.greeterLib\fP
On systems that support a dynamically-loadable greeter library, the
name of the library. The default is
diff --git a/doc/wdm.man.in b/doc/wdm.man.in
index da44f2860d6b..4c22f1db58e6 100644
--- a/doc/wdm.man.in
+++ b/doc/wdm.man.in
@@ -202,7 +202,7 @@ to pass on to the \fIXsetup\fP,
.IP \fBDisplayManager.randomFile\fP
A file to checksum to generate the seed of authorization keys.
This should be a file that changes frequently.
-The default is \fI/dev/mem\fP.
+The default is \fI/dev/urandom\fP.
.IP \fBDisplayManager.greeterLib\fP
On systems that support a dynamically-loadable greeter library, the
name of the library. The default is
diff --git a/src/wdm/genauth.c b/src/wdm/genauth.c
index e478d936be9d..3156adf8427a 100644
--- a/src/wdm/genauth.c
+++ b/src/wdm/genauth.c
@@ -71,37 +71,26 @@ longtochars (long l, unsigned char *c)
#if !defined(ARC4_RANDOM) && !defined(DEV_RANDOM)
static int
-sumFile (char *name, long sum[2])
+sumFile (char *name, long sum[], unsigned n)
{
-longbuf[1024*2];
int cnt;
int fd;
-int loops;
-int reads;
-int i;
-int ret_status = 0;
+int ret_status = 1;
fd = open (name, O_RDONLY);
if (fd < 0) {
WDMError("Cannot open randomFile \"%s\", errno = %d\n", name, errno);
return 0;
}
-#ifdef FRAGILE_DEV_MEM
-if (strcmp(name, "/dev/mem") == 0) lseek (fd, (off_t) 0x10, SEEK_SET);
-#endif
-reads = FILE_LIMIT;
-sum[0] = 0;
-sum[1] = 0;
-while ((cnt = read (fd, (char *) buf, sizeof (buf))) > 0 && --reads > 0) {
- loops = cnt / (2 * sizeof (long));
- for (i = 0; i < loops; i+= 2) {
- sum[0] += buf[i];
- sum[1] += buf[i+1];
- ret_status = 1;
- }
-}
-if (cnt < 0)
+
+memset(sum, 0, n);
+
+cnt = read(fd, (char *) sum, sizeof(long) * n);
+if (cnt < 0) {
WDMError("Cannot read randomFile \"%s\", errno = %d\n", name, errno);
+ ret_status = 0;
+}
+
close (fd);
return ret_status;
}
@@ -139,7 +128,7 @@ InitXdmcpWrapper (void)
long sum[2];
unsigned char tmpkey[8];
-if (!sumFile (randomFile, sum)) {
+if (!sumFile (randomFile, sum, 2)) {
sum[0] = time ((Time_t *) 0);
sum[1] = time ((Time_t *) 0);
}
@@ -244,7 +233,7 @@ GenerateAuthData (char *auth, int len)
localkey[0] = 1;
}
#else
- if (!sumFile (randomFile, localkey)) {
+ if (!sumFile (randomFile, localkey, 2)) {
localkey[0] = 1; /* To keep from continually calling sumFile() */
}
#endif
diff --git a/src/wdm/resource.c b/src/wdm/resource.c
index 48922c7e8b24..247819693fa4 100644
--- a/src/wdm/
Bug#700422: wdm shouldn't use /dev/mem
Hi Agustin, On Tue, Feb 12, 2013 at 07:17:53PM +0100, Agustin Martin wrote: > Thanks for your contribution. Nice to see a way to get rid of the > "program wdm tried to access /dev/mem ..." messages. Yeah, it was about time. :-) > wdm is currently orphaned and no maintainer is explicitly caring of > it, neither in Debian nor upstream. Since I made some of the final QA > non-maintainer uploads I will care of including your patch at some > time. Ok, thanks. I've tested it lightly here by building a debian package using debian/rules and applying the patch by hand (couldn't get it to apply the patch automatically with "debian/rules patch" because the patch target was missing there... yadda yadda) ... ... long story short, ping me if there's an official package available so that I can test it too, before it enters the distro repos. > Note that this will not happen soon since Debian wheezy is currently > in "frozen" state in preparation for release. Ok, that's fine. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To UNSUBSCRIBE, email to debian-qa-packages-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20130212183919.gb15...@pd.tnic
Bug#700422: wdm shouldn't use /dev/mem
On Thu, Feb 14, 2013 at 04:15:55PM +0100, Agustin Martin wrote: > I have prepared a personal package and put it in my personal Debian repo > under http://people.debian.org/~agmartin/debian-store/misc. Changes file is > signed with my Debian gpg key, available from the Debian keyring. Tested with http://people.debian.org/~agmartin/debian-store/misc/wdm_1.28-14~amd1_i386.deb here. All looks good. Thanks. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To UNSUBSCRIBE, email to debian-qa-packages-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20130214162631.ge5...@pd.tnic
