Bug 531932 - SIGSEGV booting installer with "radeon.nomodeset=1"
SIGSEGV booting installer with "radeon.nomodeset=1"
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: anaconda (Show other bugs)
rawhide
All Linux
low Severity high
: ---
: ---
Assigned To: David Cantrell
Fedora Extras Quality Assurance
: Reopened
: 532460 (view as bug list)
Depends On:
Blocks: F12AnacondaBlocker
  Show dependency treegraph
 
Reported: 2009-10-29 17:45 EDT by John Reiser
Modified: 2009-11-06 22:37 EST (History)
4 users (show)

See Also:
Fixed In Version: anaconda-12.45-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-06 22:37:33 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description John Reiser 2009-10-29 17:45:22 EDT
Description of problem: /sbin/loader aborts installation with SIGSEGV if " radeon.nomodeset=1" is added to the boot command line for "Install or upgrade an existing system"


Version-Release number of selected component (if applicable):
anaconda-12.41

How reproducible: Every time


Steps to Reproduce:
1. boot DVD for install (today's rawhide 2009-10-29)
2. hit <Tab> on "Install or upgrade an existing system"
3. append " radeon.nomodeset=1", and boot
  
Actual results:  Installation aborts with:
Greetings.
anaconda installer init version 12.41 starting
mounting /proc filesystem... done
creating /dev filesystem... done
starting udev...done
mount /dev/pts (unix98 pty) filesystem... done
mounting /sys filesystem... done
trying to remount root filesystem read write... done
mounting /tmp as tmpfs... done
running install...
running /sbin/loader
loader received SIGSEGV!  Backtrace:
/sbin/loader[0x40783e]
/lib64/libc.so.6(+0x335f0))[0x7f7a046715f0]
/sbin/loader[0x40c2f9]
/sbin/loader[0x408819]
/lib64/libc.so.6(__libc_start_main+0xfd)[0x7f7a0465cb4d]
/sbin/loader[0x4058f9]
install extited abnormally [1/1]
you may safely reboot your system


Expected results: No SIGSEGV; successful installation using radeon driver with no kernel mode setting


Additional info: The same problem occurs on i686 and x86_64.  Install via the "basic video driver" works.
Comment 1 Chris Lumens 2009-11-02 09:12:16 EST
*** Bug 532460 has been marked as a duplicate of this bug. ***
Comment 2 James Laska 2009-11-02 11:23:32 EST
Also affects booting with "drm.debug=1"
Comment 3 Adam Williamson 2009-11-02 12:20:59 EST
note that 'radeon.nomodeset=1' is an invalid parameter, you want either 'nomodeset' or 'radeon.modeset=0'. 'drm.debug=1' is valid, however, so the validity of the parameter doesn't seem to be the problem. I wonder if this happens with _all_ kernel parameters, or just ones of the module.parameter=value format which is used for passing parameters to modules...

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 4 James Laska 2009-11-02 15:26:38 EST
(In reply to comment #3)
> I wonder if this
> happens with _all_ kernel parameters, or just ones of the
> module.parameter=value format which is used for passing parameters to
> modules...

Odd, I tried booting with "foo.bar=1" and that works.  However, "drm.debug=1" and "radeon.modeset=0" both result in a SIGSEGV.
Comment 5 Ales Kozumplik 2009-11-04 13:28:41 EST
I think the cause of this problem is in the addOption() function in loader/modules.

It was changed about a month ago (29e18c35d264f93acb0bd552c287acfb753c4afb) and the current revision does not initialize modopts[i].options to anything in case the module was not found in modopts table.

When I revert the change the loader does not crash. There of course must be a reason for the previous patch, someone should contact the submitter. Unfortunately it's getting late in Brno, the bug is a blocker and I was instructed to it this over.
Comment 6 Adam Williamson 2009-11-04 13:40:28 EST
For the record, it was a dcantrell change, the commit log reads:

"Do not assume we found a module in addOption() in loader/modules.c



Avoid SIGSEGV in addOption() in loader/modules.c.  Just because found

does not equal 0 does not mean we found the module requested."

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 7 David Cantrell 2009-11-04 17:29:09 EST
modopts was not initialized under certain conditions, such as passing "radeon.nomodeset=1".  Fix:

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 *opti
     }
 
     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;
Comment 8 Adam Williamson 2009-11-04 19:58:15 EST
Tag request:

https://fedorahosted.org/rel-eng/ticket/3101

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 9 Adam Williamson 2009-11-05 13:35:23 EST
this was tagged, jlaska will test to confirm the fix and close it off.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers
Comment 10 James Laska 2009-11-05 13:44:38 EST
Confirmed the fix using anaconda-12.44-1.fc12.  Tested the following boot args:
 * radeon.modeset=0
 * foo.bar=1
 * nomodeset
 * foo.bar.baz.radeon=1
 * radeon...modeset=0

Moving this issue to CLOSED RAWHIDE.
Comment 11 David Cantrell 2009-11-06 18:57:28 EST
A point was raised on the anaconda team about my initial fix.  It does prevent SIGSEGV, but it also never collects module options, effectively ignoring them all.  I've redone the patch and would like it retested for F-12:

diff --git a/loader/modules.c b/loader/modules.c
index f61f680..fbdd0c9 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[i].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 = realloc(modopts, sizeof(*modopts) * (nummodopts + 1));
-            modopts[nummodopts].name = strdup(module);
-            modopts[nummodopts].numopts = 1;
-            modopts[nummodopts++].options = 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;
Comment 12 James Laska 2009-11-06 22:26:15 EST
Retested using http://jkeating.fedorapeople.org/boot.iso.

Tested the same boot arguments as tested before.  However, this time confirmed expected results with respect to modesetting.  The attached screenshot resolutions match what I would expect for modesetting being enabled and disabled.

 * radeon.modeset=0 - http://jlaska.fedorapeople.org/radeon.modeset=0.jpg
 * radeon.modeset=1 - http://jlaska.fedorapeople.org/radeon.modeset=1.jpg
 * nomodeset - http://jlaska.fedorapeople.org/nomodeset.jpg

In addition, the following boot arguments did not result in a sigsegv of loader.
 * foo.bar=1 
 * foo.bar.baz.radeon=1
 * radeon...modeset=0
 * asdf...foo==42
Comment 13 Adam Williamson 2009-11-06 22:37:33 EST
Looks good to me too. Confirmed that nouveau.modeset works as expected, also tried feeding all kinds of garbage in an attempt to break it and couldn't, despite going well into Doctor It Hurts territory. Unless we missed something, looks good to me.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Note You need to log in before you can comment on or make changes to this bug.