On 11/01/11 16:09, Jes.Sorensen@xxxxxxxxxx wrote:
> From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
>
> S_ISBLK() and S_ISLNK() are mutually exclusive. By swapping the checks
> round and testing S_ISBLK() first, we avoid having to silence the
> compiler for uninitialized variable usage, and avoid a warning from
> security checking tools.
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
Looking at this one again, I realized it doesn't solve the real problem.
In particular if /dev/md64 already exists as a symlink and /dev/md64p1
exists as a device node, we end up comparing against random stack data.
Please discard the previous patch and use this one instead.
Sorry for the noise.
Cheers,
Jes
>From a6411e70a8074d13f9ad0af6d68a6d8bb550c317 Mon Sep 17 00:00:00 2001
From: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
Date: Tue, 1 Nov 2011 14:53:37 +0100
Subject: [PATCH 19/19] make_parts(): Fix case of comparing against
uninitialized variables
Silencing gcc's warning of uninitialized variables was hiding a bug
where if we have /dev/md64 as a symlink, and /dev/md64p1 was a real
device node.
In this case major_num and minor_num would not get populated, but we
end up comparing against them because the stat for md64p1 succeeds.
Instead of using the int foo = foo trick, change the code to set
set the variables to invalid values so comparisons will fail.
Signed-off-by: Jes Sorensen <Jes.Sorensen@xxxxxxxxxx>
---
mdopen.c | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/mdopen.c b/mdopen.c
index 555ab84..eac1c1f 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -38,9 +38,9 @@ void make_parts(char *dev, int cnt)
* else that of dev
*/
struct stat stb;
- int major_num = major_num; /* quiet gcc -Os unitialized warning */
- int minor_num = minor_num; /* quiet gcc -Os unitialized warning */
- int odig = odig; /* quiet gcc -Os unitialized warning */
+ int major_num;
+ int minor_num;
+ int odig;
int i;
int nlen = strlen(dev) + 20;
char *name;
@@ -53,23 +53,26 @@ void make_parts(char *dev, int cnt)
if (lstat(dev, &stb)!= 0)
return;
- if (S_ISLNK(stb.st_mode)) {
+ if (S_ISBLK(stb.st_mode)) {
+ major_num = major(stb.st_rdev);
+ minor_num = minor(stb.st_rdev);
+ odig = -1;
+ } else if (S_ISLNK(stb.st_mode)) {
int len = readlink(dev, orig, sizeof(orig));
if (len < 0 || len > 1000)
return;
orig[len] = 0;
odig = isdigit(orig[len-1]);
- } else if (S_ISBLK(stb.st_mode)) {
- major_num = major(stb.st_rdev);
- minor_num = minor(stb.st_rdev);
+ major_num = -1;
+ minor_num = -1;
} else
- return;
+ return;
name = malloc(nlen);
for (i=1; i <= cnt ; i++) {
struct stat stb2;
snprintf(name, nlen, "%s%s%d", dev, dig?"p":"", i);
if (stat(name, &stb2)==0) {
- if (!S_ISBLK(stb2.st_mode))
+ if (!S_ISBLK(stb2.st_mode) || !S_ISBLK(stb.st_mode))
continue;
if (stb2.st_rdev == makedev(major_num, minor_num+i))
continue;
--
1.7.6.4
