-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Thu, 5 Nov 2009, Ales Kozumplik wrote:
On 11/04/2009 11:27 PM, David Cantrell wrote:
Under certain conditions, modopts is never initialized. This patch
fixes up that while preserving the existing functionality introduced
with 29e18c35.
---
loader/modules.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/loader/modules.c b/loader/modules.c
index d5129b7..f61f680 100644
--- a/loader/modules.c
+++ b/loader/modules.c
@@ -117,16 +117,18 @@ static void addOption(const char *module, const char
*option) {
}
if (found) {
- modopts[i].options = realloc(modopts[i].options,
- sizeof(modopts[i].options) *
- (modopts[i].numopts + 1));
- modopts[i].options[modopts[i].numopts - 1] = strdup(option);
- modopts[i].options[modopts[i].numopts] = NULL;
- } else {
- modopts = realloc(modopts, sizeof(*modopts) * (nummodopts + 1));
- modopts[nummodopts].name = strdup(module);
- modopts[nummodopts].numopts = 1;
- modopts[nummodopts++].options = NULL;
+ if (modopts == NULL) {
+ modopts = realloc(modopts, sizeof(*modopts) * (nummodopts +
1));
+ modopts[nummodopts].name = strdup(module);
+ modopts[nummodopts].numopts = 1;
+ modopts[nummodopts++].options = NULL;
+ } else {
+ modopts[i].options = realloc(modopts[i].options,
+ sizeof(modopts[i].options) *
+ (modopts[i].numopts + 1));
+ modopts[i].options[modopts[i].numopts - 1] = strdup(option);
+ modopts[i].options[modopts[i].numopts] = NULL;
+ }
}
return;
Hi again David,
I was looking at this issue yesterday and I just took a look at your patch. I
am a bit confused: The code in the new revision will do nothing in case a
module of given name is not found in the table. If we assume that nummodopts
is initially 0, nothing ever gets added and the table will remain empty -- is
that what we want?
You are correct. The addOption() function is meant to find an existing module
name and append to the option list, otherwise start a new one. My previous
patch to fix an issue on s390 failed to get that initial list created. So the
SIGSEGV we were hitting for the "radeon.nomodeset=1" parameter was actually
for the strcat() call in writeModulesConf() that iterates on
modopts[i].options and generates a module configuration.
So with that, I've recreated the patch so it works correctly now. The patch
is below.
Also: why nummodopts is initially -1 (that has nothing to do with your patch
though, just a question)? Is that because we don't want mlLoadModule calls to
modify the table before mlInitModuleConfig is called?
I have no idea. This is old code and I'm not the original author.
- From d1bd29c2a0ffe2d99366682f5ffbfb45779e674f Mon Sep 17 00:00:00 2001
From: David Cantrell <dcantrell@xxxxxxxxxx>
Date: Thu, 5 Nov 2009 11:21:04 -1000
Subject: [PATCH] Correct modopts initialization in loader (take 2) (#531932).
The edc665e6fa2ba71e89eb83412738622e916c3a05 commit prevented SIGSEGV
but changed behavior of modopts to where we'd never get any values in
modopts.
What needed to happen is the else clause in addOption() needed to
initalize the .options array and add in the option value. That wasn't
happening, so strcat() calls in writeModulesConf() were causing SIGSEGV.
This patch also adds some realloc() and malloc() checks. Tested with
"radeon.nomodeset=1" and without that parameter on x86_64.
- ---
loader/modules.c | 43 +++++++++++++++++++++++++++++++++----------
1 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/loader/modules.c b/loader/modules.c
index f61f680..a72a40a 100644
- --- a/loader/modules.c
+++ b/loader/modules.c
@@ -105,7 +105,7 @@ void mlAddBlacklist(char *module) {
}
static void addOption(const char *module, const char *option) {
- - int found = 0, i;
+ int found = 0, i, sz;
found = 0;
for (i = 0; i < nummodopts; i++) {
@@ -117,18 +117,41 @@ static void addOption(const char *module, const char *option) {
}
if (found) {
+ modopts[i].options = realloc(modopts[i].options,
+ sizeof(modopts[i].options) *
+ (modopts[i].numopts + 1));
if (modopts == NULL) {
- - modopts = realloc(modopts, sizeof(*modopts) * (nummodopts + 1));
- - modopts[nummodopts].name = strdup(module);
- - modopts[nummodopts].numopts = 1;
- - modopts[nummodopts++].options = NULL;
+ logMessage(ERROR, "%s (%d): %m", __func__, __LINE__);
+ abort();
+ }
+
+ modopts[i].options[modopts[i].numopts - 1] = strdup(option);
+ modopts[i].options[modopts[i].numopts] = NULL;
+ } else {
+ if (modopts == NULL) {
+ modopts = malloc(sizeof(struct moduleOptions) * (nummodopts + 1));
} else {
- - modopts[i].options = realloc(modopts[i].options,
- - sizeof(modopts[i].options) *
- - (modopts[i].numopts + 1));
- - modopts[i].options[modopts[i].numopts - 1] = strdup(option);
- - modopts[i].options[modopts[i].numopts] = NULL;
+ modopts = realloc(modopts, sizeof(*modopts) * (nummodopts + 1));
+ }
+
+ if (modopts == NULL) {
+ logMessage(ERROR, "%s (%d): %m", __func__, __LINE__);
+ abort();
}
+
+ modopts[nummodopts].name = strdup(module);
+ modopts[nummodopts].numopts = 1;
+
+ sz = sizeof(modopts[nummodopts].options) * 2;
+ if ((modopts[nummodopts].options = malloc(sz)) == NULL) {
+ logMessage(ERROR, "%s (%d): %m", __func__, __LINE__);
+ abort();
+ }
+
+ modopts[nummodopts].options[0] = strdup(option);
+ modopts[nummodopts].options[1] = NULL;
+
+ nummodopts++;
}
return;
- --
1.6.5.2
- --
David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iEYEARECAAYFAkrzQpUACgkQ5hsjjIy1VknTtACfXXS4pOScxb6FkbE1Qt2zgIDV
cDQAn0RWnQLkNIhe3joLbSNszLJeoOZQ
=V3xk
-----END PGP SIGNATURE-----
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
