linux-kernel - Re: [patch] HWBKPT: Make bp_len type to u64 generic across the arch

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-next>] [day] [month] [year] [list]

Date:	Sat, 30 Jan 2010 19:38:37 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Mahesh Salgaonkar <mahesh@...ux.vnet.in.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>
Cc:	Linux Kernel <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...e.hu>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	"K. Prasad" <prasad@...ux.vnet.ibm.com>,
	Maneesh Soni <maneesh@...ibm.com>,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	Martin <schwidefsky@...ibm.com>,
	Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>
Subject: Re: [patch] HWBKPT: Make bp_len type to u64 generic across the arch

On Sat, Jan 30, 2010 at 10:25:18AM +0530, Mahesh Salgaonkar wrote:
> 
> Change 'bp_len' type to __u64 to make it work across the arch.
> The s390 architecture watch point length can be upto 2^64.
> 
> reference:
> 	http://lkml.org/lkml/2010/1/25/212
> 
> Based on commit 6aa41f8b01301199af6c9febb24f3c1f5a0bc9d5
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@...ux.vnet.ibm.com>
> ---
> 
>  include/linux/hw_breakpoint.h |    2 +-
>  include/linux/perf_event.h    |    6 ++----
>  kernel/hw_breakpoint.c        |    2 +-
>  kernel/perf_event.c           |    2 +-
>  4 files changed, 5 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
> index 41235c9..76e7427 100644
> --- a/include/linux/hw_breakpoint.h
> +++ b/include/linux/hw_breakpoint.h
> @@ -44,7 +44,7 @@ static inline int hw_breakpoint_type(struct perf_event *bp)
>  	return bp->attr.bp_type;
>  }
>  
> -static inline int hw_breakpoint_len(struct perf_event *bp)
> +static inline unsigned long hw_breakpoint_len(struct perf_event *bp)
>  {
>  	return bp->attr.bp_len;
>  }



This should return a u64, or gcc will warn us about loosing
informations in 32 bits arch?



> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1438463..30c78bd 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -211,11 +211,9 @@ struct perf_event_attr {
>  		__u32		wakeup_watermark; /* bytes before wakeup   */
>  	};
>  
> -	__u32			__reserved_2;
> -
> -	__u64			bp_addr;
>  	__u32			bp_type;
> -	__u32			bp_len;
> +	__u64			bp_addr;
> +	__u64			bp_len;
>  };



Peter, what do you think about this new layout?
Putting the bp_type right after the wakeup_* fields
is going to remove the padding difference between
64 and 32 archs. That looks better than the __reserved_2
we had.

If this patch can make it for .33, it would be nice.



>  
>  /*
> diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
> index 50dbd59..a1f511f 100644
> --- a/kernel/hw_breakpoint.c
> +++ b/kernel/hw_breakpoint.c
> @@ -324,8 +324,8 @@ EXPORT_SYMBOL_GPL(register_user_hw_breakpoint);
>  int modify_user_hw_breakpoint(struct perf_event *bp, struct perf_event_attr *attr)
>  {
>  	u64 old_addr = bp->attr.bp_addr;
> +	u64 old_len = bp->attr.bp_len;
>  	int old_type = bp->attr.bp_type;
> -	int old_len = bp->attr.bp_len;
>  	int err = 0;
>  
>  	perf_event_disable(bp);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 8f8f7aa..1473cc9 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -4720,7 +4720,7 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
>  	if (attr->type >= PERF_TYPE_MAX)
>  		return -EINVAL;
>  
> -	if (attr->__reserved_1 || attr->__reserved_2)
> +	if (attr->__reserved_1)
>  		return -EINVAL;
>  
>  	if (attr->sample_type & ~(PERF_SAMPLE_MAX-1))
> 


Acked-by: Frederic Weisbecker <fweisbec@...il.com>

--
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/