Re: two patches: avoid NULL-dereference

Juliusz Chroboczek wrote:

>>> That's incorrect.
>
>> "Incorrect" ?
> [...]
>> I guess you mean "undesirable" or "contrary to policy".
>
> I s'pose I do.
>
>> Here's an alternative patch (untested):
>
>> +        if(request == NULL)
>> +            return;
>
> No, that's still "undesirable", since specialRequestHandler will never
> be invoked, and hence will never get a chance to either notify the
> client or release the object (notice the retainObject above).

Then maybe move the if/return up two lines, as below.

If not, then what do you suggest to use in place of "request"
as argument to do_stream?


>From 4a723b4ceeb06b541b8a78e37e2e0263332f3bfb Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email protected]>
Date: Wed, 19 Jan 2011 20:38:02 +0100
Subject: [PATCH] avoid another NULL-dereference upon malloc failure

* local.c (fillSpecialObject): Return early when malloc fails.
---
 local.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/local.c b/local.c
index 82106c4..5203f86 100644
--- a/local.c
+++ b/local.c
@@ -561,38 +561,40 @@ fillSpecialObject(ObjectPtr object, void (*fn)(FILE*, char*), void* closure)
         request = malloc(sizeof(SpecialRequestRec));
         if(request == NULL) {
             kill(pid, SIGTERM);
             close(filedes[0]);
             abortObject(object, 503,
                         internAtom("Couldn't allocate request\n"));
             notifyObject(object);
             /* specialRequestHandler will take care of the rest. */
         } else {
             request->buf = get_chunk();
             if(request->buf == NULL) {
                 kill(pid, SIGTERM);
                 close(filedes[0]);
                 free(request);
                 abortObject(object, 503,
                             internAtom("Couldn't allocate request\n"));
                 notifyObject(object);
             }
         }
+        if(request == NULL)
+            return;
         object->flags |= OBJECT_INPROGRESS;
         retainObject(object);
         request->object = object;
         request->fd = filedes[0];
         request->pid = pid;
         request->offset = 0;
         /* Under any sensible scheduler, the child will run before the
            parent.  So no need for IO_NOTNOW. */
         do_stream(IO_READ, filedes[0], 0, request->buf, CHUNK_SIZE,
                   specialRequestHandler, request);
     } else {
         /* child */
         close(filedes[0]);
         uninitEvents();
         do {
             rc = sigprocmask(SIG_SETMASK, &old_mask, NULL);
         } while (rc < 0 && errno == EINTR);
         if(rc < 0)
             exit(1);
--
1.7.3.5

------------------------------------------------------------------------------
Protect Your Site and Customers from Malware Attacks
Learn about various malware tactics and how to avoid them. Understand 
malware threats, the impact they can have on your business, and how you 
can protect your company and customers by using code signing.
http://p.sf.net/sfu/oracle-sfdevnl