linux-kernel - Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

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: <6dc59fa7-5885-9ed1-54c3-f2d112786312@quicinc.com>
Date:   Tue, 10 May 2022 22:44:09 +0530
From:   Charan Teja Kalla <quic_charante@...cinc.com>
To:     Christian König <christian.koenig@....com>,
        <gregkh@...uxfoundation.org>, <sumit.semwal@...aro.org>,
        <hridya@...gle.com>, <daniel.vetter@...ll.ch>,
        <tjmercier@...gle.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>,
        <dri-devel@...ts.freedesktop.org>, <linaro-mm-sig@...ts.linaro.org>
Subject: Re: [PATCH V2] dmabuf: ensure unique directory name for dmabuf stats

On 5/10/2022 8:42 PM, Christian König wrote:
>>    * The information in the interface can also be used to derive
>> per-exporter
>>    * statistics. The data from the interface can be gathered on error
>> conditions
>> @@ -172,6 +172,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>>   {
>>       struct dma_buf_sysfs_entry *sysfs_entry;
>>       int ret;
>> +    static atomic64_t unique_id = ATOMIC_INIT(0);
> 
> Please move that to the beginning of the declarations.
> 

Done. Any scripts I can run at my end to catch these type of trivial
changes? checkpatch.pl didn't report this coding style.

>>         if (!dmabuf || !dmabuf->file)
>>           return -EINVAL;
>> @@ -192,7 +193,8 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>>         /* create the directory for buffer stats */
>>       ret = kobject_init_and_add(&sysfs_entry->kobj, &dma_buf_ktype,
>> NULL,
>> -                   "%lu", file_inode(dmabuf->file)->i_ino);
>> +                   "%lu-%lu", file_inode(dmabuf->file)->i_ino,
> 
> Why not just use the unique value here? Or is the inode number necessary
> for something?

This will ease the debugging a lot. Given the dump, I can easily map
which dmabuf buffer to the process. On the crashutilty I just have to
search for this inode in the files output, just one example.