Juliusz Chroboczek wrote:
>> Subject: [PATCH 1/2] avoid NULL-dereference upon malloc failure
>
> Thanks, applied.
>
>> Subject: [PATCH 2/2] avoid another NULL-dereference upon malloc
>> failure
>
> That's incorrect.

"Incorrect" ?
Exiting is certainly better than segfaulting due to the NULL-dereference ;-)
I guess you mean "undesirable" or "contrary to policy".

>> +            do_log_error(L_ERROR, saved_errno, "Couldn't allocate 
>> request\n");
>> +            polipoExit();
>> +            return;
>
> You're not supposed to exit -- you're supposed to recover.

Well, the existing code definitely does exit-on-OOM some of the time.
I see the documentation that says there are only three such places.

Here's an alternative patch (untested):

>From c72ec2d49055f3d3b5cee2227a543da94a944ff9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@redhat.com>
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..c334b16 100644
--- a/local.c
+++ b/local.c
@@ -560,44 +560,46 @@ 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);
             }
         }
         object->flags |= OBJECT_INPROGRESS;
         retainObject(object);
+        if(request == NULL)
+            return;
         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);

         if(filedes[1] != 1)
             dup2(filedes[1], 1);

         (*fn)(stdout, closure);
--
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
_______________________________________________
Polipo-users mailing list
Polipo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/polipo-users

Reply via email to