[SSSD] [PATCH] Print pointer contents not address

Jakub Hrozek jhrozek at redhat.com
Tue Jul 10 13:10:39 UTC 2012


On Mon, Jul 09, 2012 at 07:28:12AM -0400, Stephen Gallagher wrote:
> On Sun, 2012-07-08 at 22:02 +0200, Jakub Hrozek wrote:
> > I was not able to log in today, I was getting krb5_child segfaults. The
> > attached patch fixed that.
> 
> Nack.
> 
> I fail to see how this could possibly have caused a segfault. The
> previous version would have been printing either "true" or "false" based
> on whether the pointer was NULL or contained a value. This was
> incorrect, but should be impossible to segfault on.

Yeah, the segfault was caused by incorrect format string. I'll send a
separate patch for that issue.

I assumed this was the reason because I no longer saw the segfault on my
laptop, but it turned out that the crash went away only because I tested
a build compiled with -O0, krb5_child was still crashing when compiled
with -O2.

> 
> However, the new patch changes this to a dereference without checking
> the validity of the offline pointer first. So now you've introduced the
> possibility of a segfault if the pointer is uninitialized (though it
> never should be).

We write into the address earlier anyway, so we depended on the pointer
validity. I've added a check, though.
-------------- next part --------------
>From 9b7e895f1f306cebbaeeb54c3f84f90987b7b881 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Tue, 10 Jul 2012 15:06:20 +0200
Subject: [PATCH] Print based on pointer contents not address

---
 src/providers/krb5/krb5_child.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index f647bf79e62f74ef952e53113f7e276388a01faf..8cf6255583ba95c1c35c10d121a32a96606cf2ef 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1329,6 +1329,8 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd,
 
     DEBUG(SSSDBG_TRACE_LIBS, ("total buffer size: [%d]\n", size));
 
+    if (!offline || !kr) return EINVAL;
+
     SAFEALIGN_COPY_UINT32_CHECK(&pd->cmd, buf + p, size, &p);
     SAFEALIGN_COPY_UINT32_CHECK(&kr->uid, buf + p, size, &p);
     SAFEALIGN_COPY_UINT32_CHECK(&kr->gid, buf + p, size, &p);
@@ -1344,7 +1346,7 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd,
     DEBUG(SSSDBG_CONF_SETTINGS,
           ("cmd [%d] uid [%llu] gid [%llu] validate [%s] offline [%s] "
            "UPN [%s]\n", pd->cmd, kr->uid, kr->gid,
-           kr->validate ? "true" : "false", offline ? "true" : "false",
+           kr->validate ? "true" : "false", *offline ? "true" : "false",
            kr->upn ? kr->upn : "none"));
 
     if (pd->cmd == SSS_PAM_AUTHENTICATE ||
-- 
1.7.10.4



More information about the sssd-devel mailing list