netdev - Re: [PATCH net-next] netdevsim: correctly check return value of debugfs_create

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: <20171219.091152.920867492172236638.davem@davemloft.net>
Date:   Tue, 19 Dec 2017 09:11:52 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     bhole_prashant_q7@....ntt.co.jp
Cc:     jakub.kicinski@...ronome.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] netdevsim: correctly check return value of
 debugfs_create_dir

From: "Prashant Bhole" <bhole_prashant_q7@....ntt.co.jp>
Date: Tue, 19 Dec 2017 13:45:47 +0900

> I tried to evaluate whether fixing return value of debugfs_create_dir() (and
> friends) will be useful or not because it has not been changed since very
> long time. Now I am not much convinced about changing this api. 
> 
> Important and possible error codes could be -EEXIST and -ENOMEM. Suppose
> -EEXIST is returned, IMO the directory shouldn't exists in the first place
> because it is specific to particular module. Also, there is no point in
> creating file in such directory, because directory owner (creator) might
> remove it too. This means there are less chances that api change will be
> useful. Please let me know your opinion on it.
> 
> If you are ok with above explanation, shall I submit v2 for this patch?

Well, something is seriously wrong if the directory exists already.

It could be that two netdevsim modules, independantly compiled, are trying
to be loaded.

Wouldn't it clearly be desirable to fail and not load the module in
that case?

This is why I think ignoring debugfs errors is foolish.