| From c8a139d001a1aab1ea8734db14b22dac9dd143b6 Mon Sep 17 00:00:00 2001 |
| From: NeilBrown <neilb@suse.com> |
| Date: Mon, 3 Apr 2017 11:30:34 +1000 |
| Subject: sysfs: be careful of error returns from ops->show() |
| |
| From: NeilBrown <neilb@suse.com> |
| |
| commit c8a139d001a1aab1ea8734db14b22dac9dd143b6 upstream. |
| |
| ops->show() can return a negative error code. |
| Commit 65da3484d9be ("sysfs: correctly handle short reads on PREALLOC attrs.") |
| (in v4.4) caused this to be stored in an unsigned 'size_t' variable, so errors |
| would look like large numbers. |
| As a result, if an error is returned, sysfs_kf_read() will return the |
| value of 'count', typically 4096. |
| |
| Commit 17d0774f8068 ("sysfs: correctly handle read offset on PREALLOC attrs") |
| (in v4.8) extended this error to use the unsigned large 'len' as a size for |
| memmove(). |
| Consequently, if ->show returns an error, then the first read() on the |
| sysfs file will return 4096 and could return uninitialized memory to |
| user-space. |
| If the application performs a subsequent read, this will trigger a memmove() |
| with extremely large count, and is likely to crash the machine is bizarre ways. |
| |
| This bug can currently only be triggered by reading from an md |
| sysfs attribute declared with __ATTR_PREALLOC() during the |
| brief period between when mddev_put() deletes an mddev from |
| the ->all_mddevs list, and when mddev_delayed_delete() - which is |
| scheduled on a workqueue - completes. |
| Before this, an error won't be returned by the ->show() |
| After this, the ->show() won't be called. |
| |
| I can reproduce it reliably only by putting delay like |
| usleep_range(500000,700000); |
| early in mddev_delayed_delete(). Then after creating an |
| md device md0 run |
| echo clear > /sys/block/md0/md/array_state; cat /sys/block/md0/md/array_state |
| |
| The bug can be triggered without the usleep. |
| |
| Fixes: 65da3484d9be ("sysfs: correctly handle short reads on PREALLOC attrs.") |
| Fixes: 17d0774f8068 ("sysfs: correctly handle read offset on PREALLOC attrs") |
| Signed-off-by: NeilBrown <neilb@suse.com> |
| Acked-by: Tejun Heo <tj@kernel.org> |
| Reported-and-tested-by: Miroslav Benes <mbenes@suse.cz> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/sysfs/file.c | 6 ++++-- |
| 1 file changed, 4 insertions(+), 2 deletions(-) |
| |
| --- a/fs/sysfs/file.c |
| +++ b/fs/sysfs/file.c |
| @@ -108,7 +108,7 @@ static ssize_t sysfs_kf_read(struct kern |
| { |
| const struct sysfs_ops *ops = sysfs_file_ops(of->kn); |
| struct kobject *kobj = of->kn->parent->priv; |
| - size_t len; |
| + ssize_t len; |
| |
| /* |
| * If buf != of->prealloc_buf, we don't know how |
| @@ -117,13 +117,15 @@ static ssize_t sysfs_kf_read(struct kern |
| if (WARN_ON_ONCE(buf != of->prealloc_buf)) |
| return 0; |
| len = ops->show(kobj, of->kn->priv, buf); |
| + if (len < 0) |
| + return len; |
| if (pos) { |
| if (len <= pos) |
| return 0; |
| len -= pos; |
| memmove(buf, buf + pos, len); |
| } |
| - return min(count, len); |
| + return min_t(ssize_t, count, len); |
| } |
| |
| /* kernfs write callback for regular sysfs files */ |