Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 9 Apr 2018 14:39:34 +0200
From:   Martin Schwidefsky <schwidefsky@de.ibm.com>
To:     Joe Perches <joe@perches.com>
Cc:     Heiko Carstens <heiko.carstens@de.ibm.com>,
        linux-s390@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Petr Mladek <pmladek@suse.com>
Subject: Re: s390: defective uses of va_arg in __debug_sprintf_event
In-Reply-To: <1523041723.6127.33.camel@perches.com>
References: <1523041723.6127.33.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Message-Id: <20180409143934.3f27902c@mschwideX1>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 06 Apr 2018 12:08:43 -0700
Joe Perches <joe@perches.com> wrote:

> debug_sprintf_event calls __debug_sprintf_event
> with a format and arguments.
> 
> There various types of arguments used in these
> call, but __debug_sprintf_event uses va_arg
> with only long as the type argument so random
> errors could occur because the type and argument
> are supposed to match.
> 
>     debug_entry_t *__debug_sprintf_event(debug_info_t *id, int level, char *string, ...)
>     {
>     	    [...]
>     	    va_start(ap, string);
>     	    curr_event->string = string;
>     	    for (idx = 0; idx < min(numargs, (int)(id->buf_size / sizeof(long)) - 1); idx++)
>     	    	    curr_event->args[idx] = va_arg(ap, long);
>     	    va_end(ap);
>     	    [...]
>     }
> 
> from man va_arg
> 
>     va_arg()
> 
>     if type is not compatible with the type of the actual next argument
>     (as promoted according to the default argument promotions),
>     random errors will occur.
> 
> For instance, uses like:
> 
> arch/s390/kernel/perf_cpum_sf.c:919:    debug_sprintf_event(sfdbg, 6, "pmu_enable: es=%i cs=%i ed=%i cd=%i "
> arch/s390/kernel/perf_cpum_sf.c-920-                        "tear=%p dear=%p\n", cpuhw->lsctl.es, cpuhw->lsctl.cs,
> arch/s390/kernel/perf_cpum_sf.c-921-                        cpuhw->lsctl.ed, cpuhw->lsctl.cd,
> arch/s390/kernel/perf_cpum_sf.c-922-                        (void *) cpuhw->lsctl.tear, (void *) cpuhw->lsctl.dear);
> 
> where the first 3 arguments are int but their type
> as used by va_arg in __debug_sprintf_event is long
> which could produce random errors.

In principle you are right that the va_arg handling is not 100%
correct. It works though because the C ABI for s390x requires
that arguments are sign- or zero-extended to 64 bits by the caller
of a function. This is true for values passed in registers and for
the variable argument list.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.