Le 06/09/2023 à 04:04, liulongfang a écrit :
On 2023/9/5 16:17, Herbert Xu wrote:
On Tue, Sep 05, 2023 at 07:27:47AM +0200, Marion & Christophe JAILLET wrote:
Some debugfs dir of file way be left around. Is it what your are talking
about?
Yes all allocated resources should be freed on the error path.
The other snprintf in the same file also looks suspect.
It looks correct to me.
And HPRE_DBGFS_VAL_MAX_LEN being 20, it doesn't really matter. The string
can't be truncated with just a "%u\n".
Well if you're going to go with that line of reasoning then this
case ("cluster%d") can't overflow either, no?
First, I checked the calling code of the snprintf function in all driver files in
the hisilicon directory. Only here is the processing of return value judgment.
This treatment is indeed problematic and needs to be modified.
Then, I don't quite agree with your modification plan.
The modification of this solution is not complete.
As Herbert said, ("cluster%d") may still have overflow problems.
Herbert said the contrary, and I agree with him.
HPRE_DBGFS_VAL_MAX_LEN is 20.
cluster%u will be at max:
strlen("cluster") + strlen("4294967295") + 1 = 17
(unless some system have 64 bits int?)
I do agree that it is safe to remove the test after snprintf(), but
there is no need from my POV to turn "i" into a u8.
CJ
In the end, my proposed modification scheme is this:
...
int ret;
u8 i;
for (i = 0; i < clusters_num; i++) {
snprintf(buf, HPRE_DBGFS_VAL_MAX_LEN, "cluster%u", i);
tmp_d = debugfs_create_dir(buf, qm->debug.debug_root);
...
}
...
Thanks,
Longfang.
Cheers,
