* [PATCH 01/12] mdadm: add missing break for UpdateSubarray
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 02/12] ping_monitor(): check file descriptor is valid before using and closing it Jes.Sorensen
` (11 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
mdadm.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/mdadm.c b/mdadm.c
index 56de7b7..2fe12e7 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -237,6 +237,7 @@ int main(int argc, char *argv[])
}
subarray = optarg;
}
+ break;
case UdevRules:
case 'K': if (!mode) newmode = MISC; break;
case NoSharing: newmode = MONITOR; break;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 02/12] ping_monitor(): check file descriptor is valid before using and closing it
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
2011-11-02 14:42 ` [PATCH 01/12] mdadm: add missing break for UpdateSubarray Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 03/12] mdstat_read(): Check return value of dup() before using file descriptor Jes.Sorensen
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
msg.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/msg.c b/msg.c
index 98d6d13..87d3b8d 100644
--- a/msg.c
+++ b/msg.c
@@ -207,9 +207,14 @@ int fping_monitor(int sfd)
int ping_monitor(char *devname)
{
int sfd = connect_monitor(devname);
- int err = fping_monitor(sfd);
+ int err;
+
+ if (sfd >= 0) {
+ err = fping_monitor(sfd);
+ close(sfd);
+ } else
+ err = -1;
- close(sfd);
return err;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 03/12] mdstat_read(): Check return value of dup() before using file descriptor
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
2011-11-02 14:42 ` [PATCH 01/12] mdadm: add missing break for UpdateSubarray Jes.Sorensen
2011-11-02 14:42 ` [PATCH 02/12] ping_monitor(): check file descriptor is valid before using and closing it Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 04/12] get_component_size(): Check read() return value for error before using it Jes.Sorensen
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
mdstat.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/mdstat.c b/mdstat.c
index abf6bf9..6ead24c 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -131,10 +131,15 @@ struct mdstat_ent *mdstat_read(int hold, int start)
FILE *f;
struct mdstat_ent *all, *rv, **end, **insert_here;
char *line;
+ int fd;
if (hold && mdstat_fd != -1) {
lseek(mdstat_fd, 0L, 0);
- f = fdopen(dup(mdstat_fd), "r");
+ fd = dup(mdstat_fd);
+ if (fd >= 0)
+ f = fdopen(fd, "r");
+ else
+ return NULL;
} else
f = fopen("/proc/mdstat", "r");
if (f == NULL)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 04/12] get_component_size(): Check read() return value for error before using it
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (2 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 03/12] mdstat_read(): Check return value of dup() before using file descriptor Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 05/12] array_try_spare(): open_dev() returns -1 on error, not zero Jes.Sorensen
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
sysfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sysfs.c b/sysfs.c
index 10e1597..d501792 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -387,7 +387,7 @@ unsigned long long get_component_size(int fd)
return 0;
n = read(fd, fname, sizeof(fname));
close(fd);
- if (n == sizeof(fname))
+ if (n < 0 || n == sizeof(fname))
return 0;
fname[n] = 0;
return strtoull(fname, NULL, 10) * 2;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 05/12] array_try_spare(): open_dev() returns -1 on error, not zero
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (3 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 04/12] get_component_size(): Check read() return value for error before using it Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 06/12] disk_init_and_add(): Fail if opening sysfs file descriptors fail Jes.Sorensen
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Incremental.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Incremental.c b/Incremental.c
index c2c9051..d3724a4 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -884,7 +884,7 @@ static int array_try_spare(char *devname, int *dfdp, struct dev_policy *pol,
* to obtain minimum spare size */
struct supertype *st3 = dup_super(st2);
int mdfd = open_dev(mp->devnum);
- if (!mdfd) {
+ if (mdfd < 0) {
free(st3);
goto next;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 06/12] disk_init_and_add(): Fail if opening sysfs file descriptors fail
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (4 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 05/12] array_try_spare(): open_dev() returns -1 on error, not zero Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 07/12] close_aa(): Verify file descriptors are valid before trying to close them Jes.Sorensen
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
managemon.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/managemon.c b/managemon.c
index 9e0a34d..fceaeb9 100644
--- a/managemon.c
+++ b/managemon.c
@@ -409,7 +409,13 @@ static int disk_init_and_add(struct mdinfo *disk, struct mdinfo *clone,
*disk = *clone;
disk->recovery_fd = sysfs_open(aa->devnum, disk->sys_name, "recovery_start");
+ if (disk->recovery_fd < 0)
+ return -1;
disk->state_fd = sysfs_open(aa->devnum, disk->sys_name, "state");
+ if (disk->state_fd < 0) {
+ close(disk->recovery_fd);
+ return -1;
+ }
disk->prev_state = read_dev_state(disk->state_fd);
disk->curr_state = disk->prev_state;
disk->next = aa->info.devs;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 07/12] close_aa(): Verify file descriptors are valid before trying to close them
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (5 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 06/12] disk_init_and_add(): Fail if opening sysfs file descriptors fail Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 08/12] ahci_enumerate_ports(): Don't close fd that failed to open Jes.Sorensen
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
managemon.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/managemon.c b/managemon.c
index fceaeb9..cde0d8b 100644
--- a/managemon.c
+++ b/managemon.c
@@ -117,11 +117,16 @@ static void close_aa(struct active_array *aa)
close(d->state_fd);
}
- close(aa->action_fd);
- close(aa->info.state_fd);
- close(aa->resync_start_fd);
- close(aa->metadata_fd);
- close(aa->sync_completed_fd);
+ if (aa->action_fd >= 0)
+ close(aa->action_fd);
+ if (aa->info.state_fd >= 0)
+ close(aa->info.state_fd);
+ if (aa->resync_start_fd >= 0)
+ close(aa->resync_start_fd);
+ if (aa->metadata_fd >= 0)
+ close(aa->metadata_fd);
+ if (aa->sync_completed_fd >= 0)
+ close(aa->sync_completed_fd);
}
static void free_aa(struct active_array *aa)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 08/12] ahci_enumerate_ports(): Don't close fd that failed to open
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (6 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 07/12] close_aa(): Verify file descriptors are valid before trying to close them Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 09/12] unblock_monitor(): Check sra is valid before dereferencing Jes.Sorensen
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
super-intel.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 0193fe7..b583b35 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1572,8 +1572,8 @@ static int ahci_enumerate_ports(const char *hba_path, int port_count, int host_b
printf(" (%.*s)\n", MAX_RAID_SERIAL_LEN, buf);
else
printf(" ()\n");
+ close(fd);
}
- close(fd);
free(path);
path = NULL;
}
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 09/12] unblock_monitor(): Check sra is valid before dereferencing
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (7 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 08/12] ahci_enumerate_ports(): Don't close fd that failed to open Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 10/12] Manage_ro(): Check pointer rather than dereferencing it Jes.Sorensen
` (3 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
msg.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/msg.c b/msg.c
index 87d3b8d..dc780b3 100644
--- a/msg.c
+++ b/msg.c
@@ -448,6 +448,8 @@ void unblock_monitor(char *container, const int unfreeze)
continue;
sysfs_free(sra);
sra = sysfs_read(-1, e->devnum, GET_VERSION|GET_LEVEL);
+ if (!sra)
+ continue;
if (sra->array.level > 0)
to_ping++;
if (unblock_subarray(sra, unfreeze))
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 10/12] Manage_ro(): Check pointer rather than dereferencing it
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (8 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 09/12] unblock_monitor(): Check sra is valid before dereferencing Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 11/12] fd2devname(): Don't dereference NULL pointer Jes.Sorensen
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
Manage.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Manage.c b/Manage.c
index d5110a7..d9775de 100644
--- a/Manage.c
+++ b/Manage.c
@@ -86,7 +86,7 @@ int Manage_ro(char *devname, int fd, int readonly)
sysfs_set_str(mdi, NULL, "metadata_version", vers);
cp = strchr(vers+10, '/');
- if (*cp)
+ if (cp)
*cp = 0;
ping_monitor(vers+10);
if (mdi->array.level <= 0)
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 11/12] fd2devname(): Don't dereference NULL pointer
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (9 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 10/12] Manage_ro(): Check pointer rather than dereferencing it Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 14:42 ` [PATCH 12/12] match_metadata_desc(): Fix memory leak Jes.Sorensen
2011-11-02 21:12 ` [PATCH 00/12] More leak and NULL pointer dereference fixes NeilBrown
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
super-intel.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index b583b35..f776be9 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2868,8 +2868,10 @@ static void fd2devname(int fd, char *name)
dname[rv] = '\0';
nm = strrchr(dname, '/');
- nm++;
- snprintf(name, MAX_RAID_SERIAL_LEN, "/dev/%s", nm);
+ if (nm) {
+ nm++;
+ snprintf(name, MAX_RAID_SERIAL_LEN, "/dev/%s", nm);
+ }
}
extern int scsi_get_serial(int fd, void *buf, size_t buf_len);
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 12/12] match_metadata_desc(): Fix memory leak
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (10 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 11/12] fd2devname(): Don't dereference NULL pointer Jes.Sorensen
@ 2011-11-02 14:42 ` Jes.Sorensen
2011-11-02 21:12 ` [PATCH 00/12] More leak and NULL pointer dereference fixes NeilBrown
12 siblings, 0 replies; 15+ messages in thread
From: Jes.Sorensen @ 2011-11-02 14:42 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dledford
From: Jes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
super-mbr.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/super-mbr.c b/super-mbr.c
index 5eefdf6..6499963 100644
--- a/super-mbr.c
+++ b/super-mbr.c
@@ -169,13 +169,14 @@ static void getinfo_mbr(struct supertype *st, struct mdinfo *info, char *map)
static struct supertype *match_metadata_desc(char *arg)
{
- struct supertype *st = malloc(sizeof(*st));
+ struct supertype *st;
- if (!st)
- return st;
if (strcmp(arg, "mbr") != 0)
return NULL;
+ st = malloc(sizeof(*st));
+ if (!st)
+ return st;
st->ss = &mbr;
st->info = NULL;
st->minor_version = 0;
--
1.7.6.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 00/12] More leak and NULL pointer dereference fixes
2011-11-02 14:42 [PATCH 00/12] More leak and NULL pointer dereference fixes Jes.Sorensen
` (11 preceding siblings ...)
2011-11-02 14:42 ` [PATCH 12/12] match_metadata_desc(): Fix memory leak Jes.Sorensen
@ 2011-11-02 21:12 ` NeilBrown
2011-11-03 12:40 ` Jes Sorensen
12 siblings, 1 reply; 15+ messages in thread
From: NeilBrown @ 2011-11-02 21:12 UTC (permalink / raw)
To: Jes.Sorensen; +Cc: linux-raid, dledford
[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]
On Wed, 2 Nov 2011 15:42:05 +0100 Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> Hi,
>
> As promised, here is another batch of fixes for small memory leaks,
> use of invalid file descriptors and NULL pointer dereferences.
>
> The coverity output is starting to look a lot better by now :)
>
> Cheers,
> Jes
Thanks.
>
>
> Jes Sorensen (12):
> mdadm: add missing break for UpdateSubarray
This was wasn't correct. The code was actually doing the right thing.
But I agree that it looked wrong so I rearranged the code a bit to make it
(hopefully) less confusing.
All the rest applied.
thanks,
NeilBrown
> ping_monitor(): check file descriptor is valid before using and
> closing it
> mdstat_read(): Check return value of dup() before using file
> descriptor
> get_component_size(): Check read() return value for error before
> using it
> array_try_spare(): open_dev() returns -1 on error, not zero
> disk_init_and_add(): Fail if opening sysfs file descriptors fail
> close_aa(): Verify file descriptors are valid before trying to close
> them
> ahci_enumerate_ports(): Don't close fd that failed to open
> unblock_monitor(): Check sra is valid before dereferencing
> Manage_ro(): Check pointer rather than dereferencing it
> fd2devname(): Don't dereference NULL pointer
> match_metadata_desc(): Fix memory leak
>
> Incremental.c | 2 +-
> Manage.c | 2 +-
> managemon.c | 21 ++++++++++++++++-----
> mdadm.c | 1 +
> mdstat.c | 7 ++++++-
> msg.c | 11 +++++++++--
> super-intel.c | 8 +++++---
> super-mbr.c | 7 ++++---
> sysfs.c | 2 +-
> 9 files changed, 44 insertions(+), 17 deletions(-)
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread