Subject: Re: [PATCH] iwlwifi: mvm: no need to check return value of
 debugfs_create functions
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org
Cc:     Johannes Berg <johannes.berg@intel.com>,
        Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
        Luca Coelho <luciano.coelho@intel.com>,
        Intel Linux Wireless <linuxwifi@intel.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        linux-wireless@vger.kernel.org
References: <20190122152151.16139-24-gregkh@linuxfoundation.org>
From:   Laura Abbott <labbott@redhat.com>
Message-ID: <03bb68be-8e42-a463-2d57-3be051dc2016@redhat.com>
Date:   Tue, 26 Mar 2019 16:55:54 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.0
MIME-Version: 1.0
In-Reply-To: <20190122152151.16139-24-gregkh@linuxfoundation.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 1/22/19 7:21 AM, Greg Kroah-Hartman wrote:
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
> index 33b0af24a537..c52cdc538678 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs-vif.c
> @@ -1446,9 +1446,8 @@ static ssize_t iwl_dbgfs_quota_min_read(struct file *file,
>   #define MVM_DEBUGFS_READ_WRITE_FILE_OPS(name, bufsz) \
>   	_MVM_DEBUGFS_READ_WRITE_FILE_OPS(name, bufsz, struct ieee80211_vif)
>   #define MVM_DEBUGFS_ADD_FILE_VIF(name, parent, mode) do {		\
> -		if (!debugfs_create_file(#name, mode, parent, vif,	\
> -					 &iwl_dbgfs_##name##_ops))	\
> -			goto err;					\
> +		debugfs_create_file(#name, mode, parent, vif,		\
> +				    &iwl_dbgfs_##name##_ops);		\
>   	} while (0)
>   
>   MVM_DEBUGFS_READ_FILE_OPS(mac_params);
> @@ -1483,12 +1482,6 @@ void iwl_mvm_vif_dbgfs_register(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
>   
>   	mvmvif->dbgfs_dir = debugfs_create_dir("iwlmvm", dbgfs_dir);
>   
> -	if (!mvmvif->dbgfs_dir) {
> -		IWL_ERR(mvm, "Failed to create debugfs directory under %pd\n",
> -			dbgfs_dir);
> -		return;
> -	}
> -
>   	if (iwlmvm_mod_params.power_scheme != IWL_POWER_SCHEME_CAM &&
>   	    ((vif->type == NL80211_IFTYPE_STATION && !vif->p2p) ||
>   	     (vif->type == NL80211_IFTYPE_STATION && vif->p2p)))
> @@ -1537,12 +1530,6 @@ void iwl_mvm_vif_dbgfs_register(struct iwl_mvm *mvm, struct ieee80211_vif *vif)
>   
>   	mvmvif->dbgfs_slink = debugfs_create_symlink(dbgfs_dir->d_name.name,
>   						     mvm->debugfs_dir, buf);
> -	if (!mvmvif->dbgfs_slink)
> -		IWL_ERR(mvm, "Can't create debugfs symbolic link under %pd\n",
> -			dbgfs_dir);
> -	return;
> -err:
> -	IWL_ERR(mvm, "Can't create debugfs entity\n");
>   }
>   

Fedora got a bug report https://bugzilla.redhat.com/show_bug.cgi?id=1691034
of a crash with 5.0 and the user did a bisect which pointed to ff9fb72bc077
("debugfs: return error values, not NULL") because the error checking is
no longer correct in this driver.

Based on https://patchwork.kernel.org/patch/10865839/, it looks like
this is supposed to go in for 5.2 but this needs to go in now as
the error checking is currently broken without it. Can this get queued
for Linus so we can get it in 5.0 stable?

Thanks,
Laura