* [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc
@ 2009-06-17 16:00 Jim Meyering
2009-06-19 6:11 ` Andrew Price
0 siblings, 1 reply; 5+ messages in thread
From: Jim Meyering @ 2009-06-17 16:00 UTC (permalink / raw)
To: cluster-devel.redhat.com
If the malloc of more_indir fails, the subsequent deref
via memset would cause a segfault.
I chose to avoid that by making more_indir a stack-local.
If it's 512-byte size is too big for your stack
requirements, let me know and I'll rewrite to
use malloc -- though doing it that way (and taking
care to avoid leaks), would probably end up uglier.
From fda0f39b0389088b0ea66216aed21d4f5f1bb604 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Wed, 17 Jun 2009 16:54:03 +0200
Subject: [PATCH] hexedit: avoid NULL dereference upon failed malloc
* gfs2/edit/hexedit.c (display_indirect): Avoid unchecked malloc
by declaring more_indir on the stack.
---
gfs2/edit/hexedit.c | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index e8c6030..84b4be4 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1477,13 +1477,12 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
file_offset = 0;
if (!termlines && ((level + 1 < di.di_height) ||
(S_ISDIR(di.di_mode) && !level))) {
- struct iinfo *more_indir;
int more_ind;
char *tmpbuf;
- more_indir = malloc(sizeof(struct iinfo));
tmpbuf = malloc(sbd.bsize);
if (tmpbuf) {
+ struct iinfo more_indir;
lseek(sbd.device_fd,
ind->ii[pndx].block * sbd.bsize,
SEEK_SET);
@@ -1500,18 +1499,17 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
}
memset(more_indir, 0, sizeof(struct iinfo));
if (S_ISDIR(di.di_mode)) {
- do_leaf_extended(tmpbuf, more_indir);
- display_leaf(more_indir);
+ do_leaf_extended(tmpbuf, &more_indir);
+ display_leaf(&more_indir);
} else {
more_ind = do_indirect_extended(tmpbuf,
- more_indir);
- display_indirect(more_indir,
+ &more_indir);
+ display_indirect(&more_indir,
more_ind, level + 1,
file_offset);
}
free(tmpbuf);
}
- free(more_indir);
}
print_entry_ndx = pndx; /* restore after recursion */
eol(0);
--
1.6.3.2.406.gd6a466
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc
2009-06-17 16:00 [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc Jim Meyering
@ 2009-06-19 6:11 ` Andrew Price
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Price @ 2009-06-19 6:11 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
This patch gives a build error:
hexedit.c:1500: error: incompatible type for argument 1 of ?memset?
On 17/06/09 17:00, Jim Meyering wrote:
> If the malloc of more_indir fails, the subsequent deref
> via memset would cause a segfault.
>
> I chose to avoid that by making more_indir a stack-local.
> If it's 512-byte size is too big for your stack
> requirements, let me know and I'll rewrite to
> use malloc -- though doing it that way (and taking
> care to avoid leaks), would probably end up uglier.
>
>>From fda0f39b0389088b0ea66216aed21d4f5f1bb604 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Wed, 17 Jun 2009 16:54:03 +0200
> Subject: [PATCH] hexedit: avoid NULL dereference upon failed malloc
>
> * gfs2/edit/hexedit.c (display_indirect): Avoid unchecked malloc
> by declaring more_indir on the stack.
> ---
> gfs2/edit/hexedit.c | 12 +++++-------
> 1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index e8c6030..84b4be4 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -1477,13 +1477,12 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
> file_offset = 0;
> if (!termlines && ((level + 1 < di.di_height) ||
> (S_ISDIR(di.di_mode) && !level))) {
> - struct iinfo *more_indir;
> int more_ind;
> char *tmpbuf;
>
> - more_indir = malloc(sizeof(struct iinfo));
> tmpbuf = malloc(sbd.bsize);
> if (tmpbuf) {
> + struct iinfo more_indir;
> lseek(sbd.device_fd,
> ind->ii[pndx].block * sbd.bsize,
> SEEK_SET);
> @@ -1500,18 +1499,17 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
> }
> memset(more_indir, 0, sizeof(struct iinfo));
> if (S_ISDIR(di.di_mode)) {
> - do_leaf_extended(tmpbuf, more_indir);
> - display_leaf(more_indir);
> + do_leaf_extended(tmpbuf, &more_indir);
> + display_leaf(&more_indir);
> } else {
> more_ind = do_indirect_extended(tmpbuf,
> - more_indir);
> - display_indirect(more_indir,
> + &more_indir);
> + display_indirect(&more_indir,
> more_ind, level + 1,
> file_offset);
> }
> free(tmpbuf);
> }
> - free(more_indir);
> }
> print_entry_ndx = pndx; /* restore after recursion */
> eol(0);
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc
@ 2009-06-19 7:40 Jim Meyering
2009-07-02 9:37 ` Steven Whitehouse
0 siblings, 1 reply; 5+ messages in thread
From: Jim Meyering @ 2009-06-19 7:40 UTC (permalink / raw)
To: cluster-devel.redhat.com
The patch I posted yesterday didn't even compile (missing "&" in memset)
Thanks to Andy Price for checking and reporting that.
Here's the corrected version:
From 152dbe7a8961cebf355b2f648f813e0c74b5e25f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Wed, 17 Jun 2009 16:54:03 +0200
Subject: [PATCH] hexedit: avoid NULL dereference upon failed malloc
* gfs2/edit/hexedit.c (display_indirect): Avoid unchecked malloc
by declaring more_indir on the stack.
---
gfs2/edit/hexedit.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
index e8c6030..eef2a7a 100644
--- a/gfs2/edit/hexedit.c
+++ b/gfs2/edit/hexedit.c
@@ -1477,13 +1477,12 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
file_offset = 0;
if (!termlines && ((level + 1 < di.di_height) ||
(S_ISDIR(di.di_mode) && !level))) {
- struct iinfo *more_indir;
int more_ind;
char *tmpbuf;
- more_indir = malloc(sizeof(struct iinfo));
tmpbuf = malloc(sbd.bsize);
if (tmpbuf) {
+ struct iinfo more_indir;
lseek(sbd.device_fd,
ind->ii[pndx].block * sbd.bsize,
SEEK_SET);
@@ -1498,20 +1497,19 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
(unsigned long long)ind->ii[pndx].block);
exit(-1);
}
- memset(more_indir, 0, sizeof(struct iinfo));
+ memset(&more_indir, 0, sizeof(struct iinfo));
if (S_ISDIR(di.di_mode)) {
- do_leaf_extended(tmpbuf, more_indir);
- display_leaf(more_indir);
+ do_leaf_extended(tmpbuf, &more_indir);
+ display_leaf(&more_indir);
} else {
more_ind = do_indirect_extended(tmpbuf,
- more_indir);
- display_indirect(more_indir,
+ &more_indir);
+ display_indirect(&more_indir,
more_ind, level + 1,
file_offset);
}
free(tmpbuf);
}
- free(more_indir);
}
print_entry_ndx = pndx; /* restore after recursion */
eol(0);
--
1.6.3.2.406.gd6a466
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc
2009-06-19 7:40 Jim Meyering
@ 2009-07-02 9:37 ` Steven Whitehouse
2009-07-02 11:05 ` Jim Meyering
0 siblings, 1 reply; 5+ messages in thread
From: Steven Whitehouse @ 2009-07-02 9:37 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
Also looks good to me,
Steve.
On Fri, 2009-06-19 at 09:40 +0200, Jim Meyering wrote:
> The patch I posted yesterday didn't even compile (missing "&" in memset)
> Thanks to Andy Price for checking and reporting that.
>
> Here's the corrected version:
>
> >From 152dbe7a8961cebf355b2f648f813e0c74b5e25f Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@redhat.com>
> Date: Wed, 17 Jun 2009 16:54:03 +0200
> Subject: [PATCH] hexedit: avoid NULL dereference upon failed malloc
>
> * gfs2/edit/hexedit.c (display_indirect): Avoid unchecked malloc
> by declaring more_indir on the stack.
> ---
> gfs2/edit/hexedit.c | 14 ++++++--------
> 1 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/gfs2/edit/hexedit.c b/gfs2/edit/hexedit.c
> index e8c6030..eef2a7a 100644
> --- a/gfs2/edit/hexedit.c
> +++ b/gfs2/edit/hexedit.c
> @@ -1477,13 +1477,12 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
> file_offset = 0;
> if (!termlines && ((level + 1 < di.di_height) ||
> (S_ISDIR(di.di_mode) && !level))) {
> - struct iinfo *more_indir;
> int more_ind;
> char *tmpbuf;
>
> - more_indir = malloc(sizeof(struct iinfo));
> tmpbuf = malloc(sbd.bsize);
> if (tmpbuf) {
> + struct iinfo more_indir;
> lseek(sbd.device_fd,
> ind->ii[pndx].block * sbd.bsize,
> SEEK_SET);
> @@ -1498,20 +1497,19 @@ static int display_indirect(struct iinfo *ind, int indblocks, int level, uint64_
> (unsigned long long)ind->ii[pndx].block);
> exit(-1);
> }
> - memset(more_indir, 0, sizeof(struct iinfo));
> + memset(&more_indir, 0, sizeof(struct iinfo));
> if (S_ISDIR(di.di_mode)) {
> - do_leaf_extended(tmpbuf, more_indir);
> - display_leaf(more_indir);
> + do_leaf_extended(tmpbuf, &more_indir);
> + display_leaf(&more_indir);
> } else {
> more_ind = do_indirect_extended(tmpbuf,
> - more_indir);
> - display_indirect(more_indir,
> + &more_indir);
> + display_indirect(&more_indir,
> more_ind, level + 1,
> file_offset);
> }
> free(tmpbuf);
> }
> - free(more_indir);
> }
> print_entry_ndx = pndx; /* restore after recursion */
> eol(0);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-07-02 11:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-17 16:00 [Cluster-devel] [PATCH] hexedit: avoid NULL dereference upon failed malloc Jim Meyering
2009-06-19 6:11 ` Andrew Price
-- strict thread matches above, loose matches on Subject: below --
2009-06-19 7:40 Jim Meyering
2009-07-02 9:37 ` Steven Whitehouse
2009-07-02 11:05 ` Jim Meyering
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.