[SSSD] [PATCH 5/7] responder: Use SAFEALIGN macro when checking pam data, validity.

Lukas Slebodnik lslebodn at redhat.com
Tue Nov 19 14:59:23 UTC 2013


On (15/11/13 18:29), Michal Židek wrote:
>On 11/14/2013 01:14 PM, Lukas Slebodnik wrote:>>From
>fbc7adbf1331fdb931b43f2b6ffda43565ce442e Mon Sep 17 00:00:00 2001
>>> >From: Michal Zidek<mzidek at redhat.com>
>>> >Date: Wed, 4 Sep 2013 16:17:57 +0200
>>> >Subject: [PATCH 5/7] responder: Use SAFEALIGN macro when checking
>pam data
>>> >validity.
>>> >
>>> >resolves:
>>> >https://fedorahosted.org/sssd/ticket/1359
>>> >---
>>> >src/responder/pam/pamsrv_cmd.c | 18 +++++++++++++-----
>>> >1 file changed, 13 insertions(+), 5 deletions(-)
>>> >
>>> >diff --git a/src/responder/pam/pamsrv_cmd.c
>b/src/responder/pam/pamsrv_cmd.c
>>> >index 32efc4b..6418047 100644
>>> >--- a/src/responder/pam/pamsrv_cmd.c
>>> >+++ b/src/responder/pam/pamsrv_cmd.c
>>> >@@ -144,12 +144,20 @@ static int pam_parse_in_data_v2(struct
>sss_domain_info *domains,
>>> >     uint32_t size;
>>> >     char *pam_user;
>>> >     int ret;
>>> >-    uint32_t terminator = SSS_END_OF_PAM_REQUEST;
>>> >+    uint32_t start;
>>> >+    uint32_t terminator;
>>> >
>>> >-    if (blen < 4*sizeof(uint32_t)+2 ||
>>> >-        ((uint32_t *)body)[0] != SSS_START_OF_PAM_REQUEST ||
>>> >-        memcmp(&body[blen - sizeof(uint32_t)], &terminator,
>sizeof(uint32_t)) != 0) {
>> Function memcmp is also used in the function pam_forwarder_parse_data,
>> although there is no alignment warning we should be consistent at
>least in the
>> same file.
>
>Yes. I modified it to use safealign copy as well in the new patch.
Thank you.
Works fine, warnings are gone.
ACK

>
>Michal
>
>
>PS:
>Btw. why is the condition to detect unterminated data like this
>
>if (blen >= sizeof(uint32_t)
>    && lastfourbytes != END_OF_PAM_REQUEST) {
>    return error;
>}
>
>Shouldn't it be like this?
>
>if (blen < sizeof(uint32_t)
>    || lastfourbytes != END_OF_PAM_REQUEST) {
>    return error;
>}
>
>Is it possible that pam request shorter than terminator is valid?
If you think it is a wrong you should send another patch.
I prefer simple commit which change only one thing.

LS



More information about the sssd-devel mailing list