On 3/26/19 6:26 PM, Greg Kroah-Hartman wrote:
On Tue, Mar 26, 2019 at 04:55:54PM -0700, Laura Abbott wrote:
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?
That's odd, I can't see how the error checking is wrong here. If the
directory is not created, an error will be returned, which should be
able to be handled by debugfs_create_file().
So with this patch does the error go away?
The full patch didn't apply cleanly and I didn't try to backport it
for the reporter to test. I was going off of the theory that if the
patch was there it would fix the problem.
What I _think_ is going wrong is dbgfs_dir is actually an errno value:
struct dentry *dbgfs_dir = vif->debugfs_dir;
struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
char buf[100];
/*
* Check if debugfs directory already exist before creating it.
* This may happen when, for example, resetting hw or suspend-resume
*/
if (!dbgfs_dir || mvmvif->dbgfs_dir)
return;
so this blows up in the snprintf
snprintf(buf, 100, "../../../%pd3/%pd",
dbgfs_dir,
mvmvif->dbgfs_dir);
Unless I misunderstood what the debugfs error change did. I think this
also means the if check needs to look for IS_ERR and not just !dbgfs_dir.
Thanks,
Laura
strange,
greg k-h
