ALSA: hda: Avoid racy recreation of widget kobjects

The refresh of HD-audio widget sysfs kobjects via
snd_hdac_refresh_widget_sysfs() is slightly racy.
The driver recreates the whole tree from scratch after deleting the
whole.  When CONFIG_DEBUG_KOBJECT_RELEASE option is used, kobject
release doesn't happen immediately but delayed, while the re-creation
of the same named kobject happens soon after invoking kobject_put().
This may end up with the conflicts of duplicated kobjects, as found in
the bug report below.

In this patch, we take another approach to refresh the tree: instead
of recreating the whole tree, just add the new nodes and delete the
non-existing nodes.  Since the refresh happens only once at
initialization, no longer race would happen.

Along with the code change, merge snd_hdac_refresh_widget_sysfs() with
the existing snd_hdac_refresh_widgets() with an additional bool flag
for simplifying the code.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197307
Signed-off-by: Takashi Iwai <tiwai@suse.de>
This commit is contained in:
Takashi Iwai 2017-10-18 15:51:59 +02:00
parent 28d1d6d2f3
commit 9780ded39b
5 changed files with 61 additions and 36 deletions

View File

@ -111,8 +111,7 @@ void snd_hdac_device_unregister(struct hdac_device *codec);
int snd_hdac_device_set_chip_name(struct hdac_device *codec, const char *name);
int snd_hdac_codec_modalias(struct hdac_device *hdac, char *buf, size_t size);
int snd_hdac_refresh_widgets(struct hdac_device *codec);
int snd_hdac_refresh_widget_sysfs(struct hdac_device *codec);
int snd_hdac_refresh_widgets(struct hdac_device *codec, bool sysfs);
unsigned int snd_hdac_make_cmd(struct hdac_device *codec, hda_nid_t nid,
unsigned int verb, unsigned int parm);

View File

@ -87,7 +87,7 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus,
fg = codec->afg ? codec->afg : codec->mfg;
err = snd_hdac_refresh_widgets(codec);
err = snd_hdac_refresh_widgets(codec, false);
if (err < 0)
goto error;
@ -388,11 +388,12 @@ static void setup_fg_nodes(struct hdac_device *codec)
/**
* snd_hdac_refresh_widgets - Reset the widget start/end nodes
* @codec: the codec object
* @sysfs: re-initialize sysfs tree, too
*/
int snd_hdac_refresh_widgets(struct hdac_device *codec)
int snd_hdac_refresh_widgets(struct hdac_device *codec, bool sysfs)
{
hda_nid_t start_nid;
int nums;
int nums, err;
nums = snd_hdac_get_sub_nodes(codec, codec->afg, &start_nid);
if (!start_nid || nums <= 0 || nums >= 0xff) {
@ -401,6 +402,12 @@ int snd_hdac_refresh_widgets(struct hdac_device *codec)
return -EINVAL;
}
if (sysfs) {
err = hda_widget_sysfs_reinit(codec, start_nid, nums);
if (err < 0)
return err;
}
codec->num_nodes = nums;
codec->start_nid = start_nid;
codec->end_nid = start_nid + nums;
@ -408,36 +415,6 @@ int snd_hdac_refresh_widgets(struct hdac_device *codec)
}
EXPORT_SYMBOL_GPL(snd_hdac_refresh_widgets);
/**
* snd_hdac_refresh_widget_sysfs - Reset the codec widgets and reinit the
* codec sysfs
* @codec: the codec object
*
* first we need to remove sysfs, then refresh widgets and lastly
* recreate it
*/
int snd_hdac_refresh_widget_sysfs(struct hdac_device *codec)
{
int ret;
if (device_is_registered(&codec->dev))
hda_widget_sysfs_exit(codec);
ret = snd_hdac_refresh_widgets(codec);
if (ret) {
dev_err(&codec->dev, "failed to refresh widget: %d\n", ret);
return ret;
}
if (device_is_registered(&codec->dev)) {
ret = hda_widget_sysfs_init(codec);
if (ret) {
dev_err(&codec->dev, "failed to init sysfs: %d\n", ret);
return ret;
}
}
return ret;
}
EXPORT_SYMBOL_GPL(snd_hdac_refresh_widget_sysfs);
/* return CONNLIST_LEN parameter of the given widget */
static unsigned int get_num_conns(struct hdac_device *codec, hda_nid_t nid)
{

View File

@ -414,3 +414,50 @@ void hda_widget_sysfs_exit(struct hdac_device *codec)
{
widget_tree_free(codec);
}
int hda_widget_sysfs_reinit(struct hdac_device *codec,
hda_nid_t start_nid, int num_nodes)
{
struct hdac_widget_tree *tree;
hda_nid_t end_nid = start_nid + num_nodes;
hda_nid_t nid;
int i;
if (!codec->widgets)
return hda_widget_sysfs_init(codec);
tree = kmemdup(codec->widgets, sizeof(*tree), GFP_KERNEL);
if (!tree)
return -ENOMEM;
tree->nodes = kcalloc(num_nodes + 1, sizeof(*tree->nodes), GFP_KERNEL);
if (!tree->nodes) {
kfree(tree);
return -ENOMEM;
}
/* prune non-existing nodes */
for (i = 0, nid = codec->start_nid; i < codec->num_nodes; i++, nid++) {
if (nid < start_nid || nid >= end_nid)
free_widget_node(codec->widgets->nodes[i],
&widget_node_group);
}
/* add new nodes */
for (i = 0, nid = start_nid; i < num_nodes; i++, nid++) {
if (nid < codec->start_nid || nid >= codec->end_nid)
add_widget_node(tree->root, nid, &widget_node_group,
&tree->nodes[i]);
else
tree->nodes[i] =
codec->widgets->nodes[nid - codec->start_nid];
}
/* replace with the new tree */
kfree(codec->widgets->nodes);
kfree(codec->widgets);
codec->widgets = tree;
kobject_uevent(tree->root, KOBJ_CHANGE);
return 0;
}

View File

@ -28,6 +28,8 @@ static inline unsigned int get_wcaps_channels(u32 wcaps)
extern const struct attribute_group *hdac_dev_attr_groups[];
int hda_widget_sysfs_init(struct hdac_device *codec);
int hda_widget_sysfs_reinit(struct hdac_device *codec, hda_nid_t start_nid,
int num_nodes);
void hda_widget_sysfs_exit(struct hdac_device *codec);
#endif /* __HDAC_LOCAL_H */

View File

@ -977,7 +977,7 @@ int snd_hda_codec_update_widgets(struct hda_codec *codec)
hda_nid_t fg;
int err;
err = snd_hdac_refresh_widget_sysfs(&codec->core);
err = snd_hdac_refresh_widgets(&codec->core, true);
if (err < 0)
return err;