linux-kernel - Re: [Cbe-oss-dev] [RFC, PATCH] CELL Oprofile SPU profiling updated patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <200702280244.12844.arnd@arndb.de>
Date:	Wed, 28 Feb 2007 02:44:12 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	cbe-oss-dev@...abs.org
Cc:	Maynard Johnson <maynardj@...ibm.com>,
	Gerhard Stenzel <gerhard.stenzel@...ibm.com>,
	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org,
	Mike Perks <mperks@...ibm.com>,
	oprofile-list@...ts.sourceforge.net
Subject: Re: [Cbe-oss-dev] [RFC, PATCH] CELL Oprofile SPU profiling updated patch

On Tuesday 27 February 2007, Maynard Johnson wrote:
> I have applied the "cleanup" patch that Arnd sent, but had to fix up a 
> few things:
>    -  Bug fix:  Initialize retval in spu_task_sync.c, line 95, otherwise 
> OProfile this function returns non-zero and OProfile fails.
>    -  Remove unused codes in include/linux/oprofile.h
>    -  Compile warnings:  Initialize offset and spu_cookie at lines 283 
> and 284 in spu_task_sync.c
> 
> With these changes and some userspace changes that were necessary to 
> correspond with Arnd's changes, our testing was successful.
> 
> A fixup patch is attached.
> 

The patch does not contain any of the metadata I need to apply it
(subject, description, signed-off-by).

> @@ -280,8 +280,8 @@ static int process_context_switch(struct
>  {
>         unsigned long flags;
>         int retval;
> -       unsigned int offset;
> -       unsigned long spu_cookie, app_dcookie;
> +       unsigned int offset = 0;
> +       unsigned long spu_cookie = 0, app_dcookie;
>         retval = prepare_cached_spu_info(spu, objectId);
>         if (retval)
>                 goto out;

No, this is wrong. Leaving the variables uninitialized at least warns
you about the bug you have in this function: when there is anything wrong,
you just continue writing the record with zero offset and dcookie values
in it. Instead, you should get handle the error condition somewhere
down the code.

It's harmless most of the time, but you really should not be painting
over your bugs by blindly initializing variables.

> diff -paur linux-orig/include/linux/oprofile.h linux-new/include/linux/oprofile.h
> --- linux-orig/include/linux/oprofile.h 2007-02-27 14:41:29.000000000 -0600
> +++ linux-new/include/linux/oprofile.h  2007-02-27 14:43:18.000000000 -0600
> @@ -36,9 +36,6 @@
>  #define XEN_ENTER_SWITCH_CODE          10
>  #define SPU_PROFILING_CODE             11
>  #define SPU_CTX_SWITCH_CODE            12
> -#define SPU_OFFSET_CODE                13
> -#define SPU_COOKIE_CODE                14
> -#define SPU_SHLIB_COOKIE_CODE          15
>  
>  struct super_block;
>  struct dentry;
> 
Right, I forgot about this.

	Arnd <><

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/