Bug 142954

Summary: xdm greeter - unhandled error case for malloc
Product: [openSUSE] SUSE LINUX 10.0 Reporter: Jan Engelhardt <jengelh>
Component: X.OrgAssignee: Matthias Hopf <mhopf>
Status: VERIFIED FIXED QA Contact: Stefan Dirsch <sndirsch>
Severity: Normal    
Priority: P3 - Medium CC: eich, sndirsch, werner
Version: RC 4   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Found By: Beta-Customer Services Priority:
Business Priority: Blocker: ---
Marketing QA Status: --- IT Deployment: ---
Attachments: Patch to catch realloc=>NULL and malloc=>NULL cases

Description Jan Engelhardt 2006-01-12 21:45:58 UTC
In case of an out-of-memory condition, xdm's greeter will probably segfault.
Here is a patch to fix the potential segfault.
Comment 1 Jan Engelhardt 2006-01-12 21:46:30 UTC
Created attachment 63188 [details]
Patch to catch realloc=>NULL and malloc=>NULL cases
Comment 2 Stefan Dirsch 2006-01-12 21:54:15 UTC
> +                               if((reply = realloc(reply, size)) == NULL) {
> +                                    free(reply);

free(NULL) ?
Comment 3 Jan Engelhardt 2006-01-12 22:23:14 UTC
Hm, right. Should have been something like:


void *r2 = reply;
if((reply = realloc(reply, ...) == NULL)
    free(r2);

How meaningful is free()ing anyway? Does not xdm greet exit somehow anyway and
therefore automatically have any leaking memory relinquished? That would save
us the free()s.
Comment 4 Jan Engelhardt 2006-01-12 22:23:30 UTC
-
Comment 5 Stefan Dirsch 2006-01-13 05:39:47 UTC
> How meaningful is free()ing anyway? Does not xdm greet exit somehow anyway 
> and therefore automatically have any leaking memory relinquished? That would
> save us the free()s.

At least it's not done at this location in the original code.
Comment 10 Matthias Hopf 2006-01-19 15:11:00 UTC
I'll check the code upstream.
Comment 11 Stefan Dirsch 2006-01-19 15:15:29 UTC
Thanks!
Comment 12 Egbert Eich 2006-01-19 16:11:11 UTC
Bailing out at an out-of-memory seems to be something that we should put upstream
- even if this is just fixing a corner case.
Comment 13 Matthias Hopf 2006-03-01 15:48:34 UTC
Fixed upstream.
Comment 14 Jan Engelhardt 2006-12-02 17:06:54 UTC
comment #2: free(reply/NULL) is allowed, though it does not make much sense when the argument is known to be NULL, of course.