linux-kernel - Re: [PATCH 02/17] coresight tmc: Hide trace buffer handling for file read

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]

Message-ID: <e667552a-c763-dac1-cd49-6e973df765a6@arm.com>
Date:   Wed, 1 Nov 2017 09:55:06 +0000
From:   Suzuki K Poulose <Suzuki.Poulose@....com>
To:     Julien Thierry <julien.thierry@....com>,
        linux-arm-kernel@...ts.infradead.org
Cc:     mathieu.poirier@...aro.org, coresight@...ts.linaro.org,
        linux-kernel@...r.kernel.org, rob.walker@....com,
        mike.leach@...aro.org
Subject: Re: [PATCH 02/17] coresight tmc: Hide trace buffer handling for file
 read

On 20/10/17 13:34, Julien Thierry wrote:
> Hi Suzuki,
> 
> On 19/10/17 18:15, Suzuki K Poulose wrote:
>> At the moment we adjust the buffer pointers for reading the trace
>> data via misc device in the common code for ETF/ETB and ETR. Since
>> we are going to change how we manage the buffer for ETR, let us
>> move the buffer manipulation to the respective driver files, hiding
>> it from the common code. We do so by adding type specific helpers
>> for finding the length of data and the pointer to the buffer,
>> for a given length at a file position.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@...aro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> ---
>>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 16 ++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 33 ++++++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tmc.c     | 34 ++++++++++++++-----------
>>   drivers/hwtracing/coresight/coresight-tmc.h     |  4 +++
>>   4 files changed, 72 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index e2513b786242..0b6f1eb746de 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -120,6 +120,22 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
>>       CS_LOCK(drvdata->base);
>>   }
>> +/*
>> + * Return the available trace data in the buffer from @pos, with
>> + * a maximum limit of @len, updating the @bufpp on where to
>> + * find it.
>> + */
>> +ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata,
>> +                  loff_t pos, size_t len, char **bufpp)
>> +{
>> +    /* Adjust the len to available size @pos */
>> +    if (pos + len > drvdata->len)
>> +        len = drvdata->len - pos;
>> +    if (len > 0)
> 
> Do we have some guarantee that "pos <= drvdata->len"? because since len is unsigned this check only covers the case were len is 0.
> 
> Maybe it would be better to use a signed variable to store the result of the difference.
> 
>> +        *bufpp = drvdata->buf + pos;


>> + * Return the available trace data in the buffer @pos, with a maximum
>> + * limit of @len, also updating the @bufpp on where to find it.
>> + */
>> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
>> +                loff_t pos, size_t len, char **bufpp)
>> +{
>> +    char *bufp = drvdata->buf + pos;
>> +    char *bufend = (char *)(drvdata->vaddr + drvdata->size);
>> +
>> +    /* Adjust the len to available size @pos */
>> +    if (pos + len > drvdata->len)
>> +        len = drvdata->len - pos;
>> +
>> +    if (len <= 0)
>> +        return len;
> 
> Similar issue here.

Thanks for spotting. I will fix it.

Cheers