linux-kernel - Re: powerpc/perf/hv-gpci: Increase request buffer size

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

Date:	Tue,  9 Feb 2016 20:45:18 +1100 (AEDT)
From:	Michael Ellerman <mpe@...erman.id.au>
To:	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
Cc:	linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: powerpc/perf/hv-gpci: Increase request buffer size

On Tue, 2016-09-02 at 03:08:30 UTC, Sukadev Bhattiprolu wrote:
> >From 31edd352fb7c2a72913f1977fa1bf168109089ad Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
> Date: Tue, 9 Feb 2016 02:47:45 -0500
> Subject: [PATCH] powerpc/perf/hv-gpci: Increase request buffer size
> 
> The GPCI hcall allows for a 4K buffer but we limit the buffer
> to 1K. The problem with a 1K buffer is if a request results in
> returning more values than can be accomodated in the 1K buffer
> the request will fail.
> 
> The buffer we are using is currently allocated on the stack and
> hence limited in size. Instead use a per-CPU 4K buffer like we do
> with 24x7 counters (hv-24x7.c).
> 
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> index 856fe6e..e6fad73 100644
> --- a/arch/powerpc/perf/hv-gpci.c
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -127,8 +127,16 @@ static const struct attribute_group *attr_groups[] = {
>  	NULL,
>  };
>  
> +#define HGPCI_REQ_BUFFER_SIZE	4096
>  #define GPCI_MAX_DATA_BYTES \
> -	(1024 - sizeof(struct hv_get_perf_counter_info_params))
> +	(HGPCI_REQ_BUFFER_SIZE - sizeof(struct hv_get_perf_counter_info_params))
> +
> +DEFINE_PER_CPU(char, hv_gpci_reqb[HGPCI_REQ_BUFFER_SIZE]) __aligned(sizeof(uint64_t));
> +
> +struct hv_gpci_request_buffer {
> +	struct hv_get_perf_counter_info_params params;
> +	uint8_t bytes[1];

bytes is 1 byte long, but ..

> @@ -163,9 +168,11 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index,
>  	 */
>  	count = 0;
>  	for (i = offset; i < offset + length; i++)
> -		count |= arg.bytes[i] << (i - offset);
> +		count |= arg->bytes[i] << (i - offset);


Here you read from bytes[i] where i can be > 1 (AFAICS).

That's fishy at best, and newer GCCs just don't allow it.

I think you could do this and it would work, but untested:

   struct hv_gpci_request_buffer {
   	struct hv_get_perf_counter_info_params params;
   	uint8_t bytes[4096 - sizeof(struct hv_get_perf_counter_info_parms)];
   };

cheers