Re: [PATCH 3/4] media: i2c: Add TDA1997x HDMI receiver driver
Hi Tim, On 23/09/17 00:24, Tim Harvey wrote: > Add support for the TDA1997x HDMI receivers. I did a very quick high-level scan and found a few blockers: 1) You *must* implement the get/set_edid ops. I won't accept the driver without that. You can use v4l2-ctl to set/get the EDID (see v4l2-ctl --help-edid). 2) The dv_timings_cap and enum_dv_timings ops are missing: those must be implemented as well. 3) Drop the deprecated g_mbus_config op. 4) Do a proper implementation of query_dv_timings. It appears you change the timings in the irq function: that's wrong. The sequence should be the following: a) the irq handler detects that timings have changed and sends a V4L2_EVENT_SOURCE_CHANGE event to userspace. b) when userspace receives that event it will stop streaming, call VIDIOC_QUERY_DV_TIMINGS and if new valid timings are detected it will call VIDIOC_S_DV_TIMINGS, allocate the new buffers and start streaming again. The driver shall never switch timings on its own, this must be directed from userspace. Different timings will require different configuration through the whole stack (other HW in the video pipeline, DMA engines, userspace memory allocations, etc, etc). Only userspace can do the reconfiguration. General note: if you want to implement CEC and/or HDCP, contact me first. I can give pointers on how to do that. This is just a quick scan. I won't have time to do an in-depth review for the next two weeks. Ideally you'll have a v2 ready by then with the issues mentioned above fixed. Did you run the v4l2-compliance utility to test this driver? For a v2 please run it and add the output to the cover letter of the patch series. You say "TDA19972 support (2 inputs)": I assume that that means that there are 2 inputs, but only one is active at a time. Right? Regards, Hans > > Signed-off-by: Tim Harvey > --- > drivers/media/i2c/Kconfig|9 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/tda1997x.c | 3065 > ++ > include/dt-bindings/media/tda1997x.h | 78 + > include/media/i2c/tda1997x.h | 53 + > 5 files changed, 3206 insertions(+) > create mode 100644 drivers/media/i2c/tda1997x.c > create mode 100644 include/dt-bindings/media/tda1997x.h > create mode 100644 include/media/i2c/tda1997x.h
Re: [PATCH 0/8] [media] v4l2-core: Fine-tuning for some function implementations
On 22/09/17 22:08, SF Markus Elfring wrote: >> Sorry Markus, just stay away from the videobuf-* sources. > > Will the software evolution be continued for related source files? > Are there any update candidates left over in the directory “v4l2-core”? Sorry, I don't understand the question. We don't want to touch the videobuf-* files unless there is a very good reason. That old videobuf framework is deprecated and the code is quite fragile (i.e. easy to break things). Everything else in that directory is under continuous development. It's core code though, so it gets a much more in-depth code review than patches for e.g. a driver. Regards, Hans
[PATCH v3] staging: greybus: light: Release memory obtained by kasprintf
Free memory region, if gb_lights_channel_config is not successful. Signed-off-by: Arvind Yadav --- changes in v2: - Subject line changed. - add kfree in __gb_lights_led_unregister(). - No need to check return value of gb_lights_channel_flash_config(). changes ib v3: - separate patch for "No need to check return value of gb_lights_channel_flash_config()". drivers/staging/greybus/light.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index 3f4148c..0f538b8 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -925,6 +925,8 @@ static void __gb_lights_led_unregister(struct gb_channel *channel) return; led_classdev_unregister(cdev); + kfree(cdev->name); + cdev->name = NULL; channel->led = NULL; } -- 2.7.4
[PATCH] staging: greybus: light: remove unnecessary error check
It is not necessary to check return value of gb_lights_channel_flash_config. gb_lights_channel_config returns both successful and error value. Signed-off-by: Arvind Yadav --- drivers/staging/greybus/light.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c index 0f538b8..d7da475 100644 --- a/drivers/staging/greybus/light.c +++ b/drivers/staging/greybus/light.c @@ -1000,11 +1000,7 @@ static int gb_lights_channel_config(struct gb_light *light, light->has_flash = true; - ret = gb_lights_channel_flash_config(channel); - if (ret < 0) - return ret; - - return ret; + return gb_lights_channel_flash_config(channel); } static int gb_lights_light_config(struct gb_lights *glights, u8 id) -- 2.7.4
Re: [v8 0/4] cgroup-aware OOM killer
On Fri, 22 Sep 2017, Tejun Heo wrote: > > If you have this low priority maintenance job charging memory to the high > > priority hierarchy, you're already misconfigured unless you adjust > > /proc/pid/oom_score_adj because it will oom kill any larger process than > > itself in today's kernels anyway. > > > > A better configuration would be attach this hypothetical low priority > > maintenance job to its own sibling cgroup with its own memory limit to > > avoid exactly that problem: it going berserk and charging too much memory > > to the high priority container that results in one of its processes > > getting oom killed. > > And how do you guarantee that across delegation boundaries? The > points you raise on why the priority should be applied level-by-level > are exactly the same points why this doesn't really work. OOM killing > priority isn't something which can be distributed across cgroup > hierarchy level-by-level. The resulting decision tree doesn't make > any sense. > It works very well in practice with real world usecases, and Roman has developed the same design independently that we have used for the past four years. Saying it doesn't make any sense doesn't hold a lot of weight when we both independently designed and implemented the same solution to address our usecases. > I'm not against adding something which works but strict level-by-level > comparison isn't the solution. > Each of the eight versions of Roman's cgroup aware oom killer has done comparisons between siblings at each level. Userspace influence on that comparison would thus also need to be done at each level. It's a very powerful combination in practice. Thanks.
[GIT PULL] parisc architecture fixes for v4.14-rc2
Hi Linus, please pull fixes for the parisc architecture for kernel 4.14-rc2 from: git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git parisc-4.14-2 Changelog: - Unbreak parisc bootloader by avoiding a gcc-7 optimization to convert multiple byte-accesses into one word-access. - Add missing HWPOISON page fault handler code. I completely missed that when I added HWPOISON support during this merge window and it only showed up now with the madvise07 LTP test case. - Fix backtrace unwinding to stop when stack start has been reached. - Issue warning if initrd has been loaded into memory regions with broken RAM modules. - Fix HPMC handler (parisc hardware fault handler) to comply with architecture specification. - Avoid compiler warnings about too large frame sizes. - Minor init-section fixes. Thanks, Helge Helge Deller (10): parisc: Fix too large frame size warnings parisc: Stop unwinding at start of stack parisc: Move start_parisc() into init section parisc: Add wrapper for pdc_instr() firmware function parisc: Add PDCE_CHECK instruction to HPMC handler parisc: Check if initrd was loaded into broken RAM parisc: Move init_per_cpu() into init section parisc: Add HWPOISON page fault handler code parisc: Reintroduce option to gzip-compress the kernel parisc: Unbreak bootloader due to gcc-7 optimizations arch/parisc/Kconfig | 12 arch/parisc/Makefile | 5 + arch/parisc/boot/compressed/Makefile | 2 +- arch/parisc/boot/compressed/misc.c | 3 ++- arch/parisc/include/asm/pdc.h| 1 + arch/parisc/include/asm/smp.h| 1 + arch/parisc/kernel/firmware.c| 20 arch/parisc/kernel/pdt.c | 9 + arch/parisc/kernel/processor.c | 2 +- arch/parisc/kernel/setup.c | 6 +++--- arch/parisc/kernel/smp.c | 3 +-- arch/parisc/kernel/traps.c | 10 +- arch/parisc/kernel/unwind.c | 12 arch/parisc/mm/fault.c | 33 + lib/Kconfig.debug| 3 ++- 15 files changed, 108 insertions(+), 14 deletions(-)
Re: [PATCH] perf report: Fix debug messages with --call-graph option
Hi Jiri,
Thanks for the review. Agreed, the patch seems confused.
I'll add more details in the next version patch and resend it later.
Thanks,
Mengting Zhang
On 2017/9/22 17:18, Jiri Olsa wrote:
On Fri, Sep 22, 2017 at 10:06:53AM +0800, zhangmengting wrote:
With --call-graph option, perf report can display call chains using
type, min percent threshold, optional print limit and order. And the
default call-graph parameter is 'graph,0.5,caller,function,percent'.
Before this patch, 'perf report --call-graph' shows incorrect debug
messages as below:
[root@localhost perf]# ./perf report --call-graph
Invalid callchain mode: 0.5
Invalid callchain order: 0.5
Invalid callchain sort key: 0.5
Invalid callchain config key: 0.5
Invalid callchain mode: caller
Invalid callchain mode: function
Invalid callchain order: function
Invalid callchain mode: percent
Invalid callchain order: percent
Invalid callchain sort key: percent
hum, could you please mention what was wrong with
the code and why the change fixes them?
looks like you just moved the warning out of the
way of the report
thanks,
jirka
The patch fixes this issue.
Signed-off-by: zhangmengting
---
tools/perf/util/callchain.c | 35 +--
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 510b513..be09d77 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -65,8 +65,6 @@ static int parse_callchain_mode(const char *value)
callchain_param.mode = CHAIN_FOLDED;
return 0;
}
-
- pr_err("Invalid callchain mode: %s\n", value);
return -1;
}
@@ -82,8 +80,6 @@ static int parse_callchain_order(const char *value)
callchain_param.order_set = true;
return 0;
}
-
- pr_err("Invalid callchain order: %s\n", value);
return -1;
}
@@ -105,8 +101,6 @@ static int parse_callchain_sort_key(const char *value)
callchain_param.branch_callstack = 1;
return 0;
}
-
- pr_err("Invalid callchain sort key: %s\n", value);
return -1;
}
@@ -124,8 +118,6 @@ static int parse_callchain_value(const char *value)
callchain_param.value = CCVAL_COUNT;
return 0;
}
-
- pr_err("Invalid callchain config key: %s\n", value);
return -1;
}
@@ -319,12 +311,27 @@ int perf_callchain_config(const char *var, const char *value)
return ret;
}
- if (!strcmp(var, "print-type"))
- return parse_callchain_mode(value);
- if (!strcmp(var, "order"))
- return parse_callchain_order(value);
- if (!strcmp(var, "sort-key"))
- return parse_callchain_sort_key(value);
+ if (!strcmp(var, "print-type")){
+ int ret;
+ ret = parse_callchain_mode(value);
+ if (ret == -1)
+ pr_err("Invalid callchain mode: %s\n", value);
+ return ret;
+ }
+ if (!strcmp(var, "order")){
+ int ret;
+ ret = parse_callchain_order(value);
+ if (ret == -1)
+ pr_err("Invalid callchain order: %s\n", value);
+ return ret;
+ }
+ if (!strcmp(var, "sort-key")){
+ int ret;
+ ret = parse_callchain_sort_key(value);
+ if (ret == -1)
+ pr_err("Invalid callchain sort key: %s\n", value);
+ return ret;
+ }
if (!strcmp(var, "threshold")) {
callchain_param.min_percent = strtod(value, &endptr);
if (value == endptr) {
--
1.7.12.4
--
To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
.
[PATCH v2] perf report: Fix debug messages with --call-graph option
With --call-graph option, perf report can display call chains using
type, min percent threshold, optional print limit and order. And the
default call-graph parameter is 'graph,0.5,caller,function,percent'.
Before this patch, 'perf report --call-graph' shows incorrect debug
messages as below:
[root@localhost perf]# ./perf report --call-graph
Invalid callchain mode: 0.5
Invalid callchain order: 0.5
Invalid callchain sort key: 0.5
Invalid callchain config key: 0.5
Invalid callchain mode: caller
Invalid callchain mode: function
Invalid callchain order: function
Invalid callchain mode: percent
Invalid callchain order: percent
Invalid callchain sort key: percent
That is because in function __parse_callchain_report_opt(),each field
of the call-graph parameter is passed to parse_callchain_{mode,order,
sort_key,value} in turn until it meets the matching value.
For example, the order field "caller" is passed to parse_callchain_mode()
firstly and obviously it doesn't match any mode field. Therefore
parse_callchain_mode() will shows the debug message "Invalid callchain
mode: caller", which could confuse users.
The patch fixes this issue by moving the warning out of the function
parse_callchain_{mode,order,sort_key,value}.
Signed-off-by: Mengting Zhang
---
tools/perf/util/callchain.c | 35 +--
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 510b513..be09d77 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -65,8 +65,6 @@ static int parse_callchain_mode(const char *value)
callchain_param.mode = CHAIN_FOLDED;
return 0;
}
-
- pr_err("Invalid callchain mode: %s\n", value);
return -1;
}
@@ -82,8 +80,6 @@ static int parse_callchain_order(const char *value)
callchain_param.order_set = true;
return 0;
}
-
- pr_err("Invalid callchain order: %s\n", value);
return -1;
}
@@ -105,8 +101,6 @@ static int parse_callchain_sort_key(const char *value)
callchain_param.branch_callstack = 1;
return 0;
}
-
- pr_err("Invalid callchain sort key: %s\n", value);
return -1;
}
@@ -124,8 +118,6 @@ static int parse_callchain_value(const char *value)
callchain_param.value = CCVAL_COUNT;
return 0;
}
-
- pr_err("Invalid callchain config key: %s\n", value);
return -1;
}
@@ -319,12 +311,27 @@ int perf_callchain_config(const char *var, const char
*value)
return ret;
}
- if (!strcmp(var, "print-type"))
- return parse_callchain_mode(value);
- if (!strcmp(var, "order"))
- return parse_callchain_order(value);
- if (!strcmp(var, "sort-key"))
- return parse_callchain_sort_key(value);
+ if (!strcmp(var, "print-type")){
+ int ret;
+ ret = parse_callchain_mode(value);
+ if (ret == -1)
+ pr_err("Invalid callchain mode: %s\n", value);
+ return ret;
+ }
+ if (!strcmp(var, "order")){
+ int ret;
+ ret = parse_callchain_order(value);
+ if (ret == -1)
+ pr_err("Invalid callchain order: %s\n", value);
+ return ret;
+ }
+ if (!strcmp(var, "sort-key")){
+ int ret;
+ ret = parse_callchain_sort_key(value);
+ if (ret == -1)
+ pr_err("Invalid callchain sort key: %s\n", value);
+ return ret;
+ }
if (!strcmp(var, "threshold")) {
callchain_param.min_percent = strtod(value, &endptr);
if (value == endptr) {
--
1.7.12.4
[PATCH] Revert "f2fs: node segment is prior to data segment selected victim"
This reverts commit b9cd20619e359d199b755543474c3d853c8e3415.
That patch causes much fewer node segments (which can be used for SSR)
than before, and in the corner case (e.g. create and delete *.txt files in
one same directory, there will be very few node segments but many data
segments), if the reserved free segments are all used up during gc, then
the write_checkpoint can still flush dentry pages to data ssr segments,
but will probably fail to flush node pages to node ssr segments, since
there are not enough node ssr segments left (the left ones are all
full).
So revert this patch to give a fair chance to let node segments remain
for SSR, which provides more robustness for corner cases.
Conflicts:
fs/f2fs/gc.c
---
fs/f2fs/gc.c | 12 +---
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index bfe6a8c..f777e07 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -267,16 +267,6 @@ static unsigned int get_cb_cost(struct f2fs_sb_info *sbi,
unsigned int segno)
return UINT_MAX - ((100 * (100 - u) * age) / (100 + u));
}
-static unsigned int get_greedy_cost(struct f2fs_sb_info *sbi,
- unsigned int segno)
-{
- unsigned int valid_blocks =
- get_valid_blocks(sbi, segno, true);
-
- return IS_DATASEG(get_seg_entry(sbi, segno)->type) ?
- valid_blocks * 2 : valid_blocks;
-}
-
static inline unsigned int get_gc_cost(struct f2fs_sb_info *sbi,
unsigned int segno, struct victim_sel_policy *p)
{
@@ -285,7 +275,7 @@ static inline unsigned int get_gc_cost(struct f2fs_sb_info
*sbi,
/* alloc_mode == LFS */
if (p->gc_mode == GC_GREEDY)
- return get_greedy_cost(sbi, segno);
+ return get_valid_blocks(sbi, segno, true);
else
return get_cb_cost(sbi, segno);
}
--
1.8.5.2
[Question] null pointer risk of kernel workqueue
Hi Tejun & Jiangshan, I find an null pointer risk in the code of workqueue. Here is description: If draining, __queue_work() will call the function is_chained_work() to do some checks. In is_chained_work(), worker->current_pwq is used directly. It should be not safe. http://elixir.free-electrons.com/linux/latest/source/kernel/workqueue.c#L1384 If you check the thread function of this worker, worker_thread(), you will find worker->current_pwq is null when one work is done or ready to be processed. This issue may happen only if we queue work during executing drain_workqueue(). http://elixir.free-electrons.com/linux/latest/source/kernel/workqueue.c#L2173 There are very few places to call drain_workqueue() in the whole linux kernel. I think that's why no one noticed this risk. Xiaofei Tan ___ linuxarm mailing list linux...@huawei.com http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm .
Re: [PATCH v4 0/3] x86/fpu: prevent leaking FPU registers via invalid FPU state
* Eric Biggers wrote: > From: Eric Biggers > > This series fixes the bug found by syzkaller where the ptrace syscall > can be used to set invalid bits in a task's FPU state. I also found > that an equivalent bug was reachable using the sigreturn syscall, so the > first patch fixes the bug in both cases. > > The other two patches start validating the other parts of the > xstate_header and make it so that invalid FPU states can no longer be > abused to leak the FPU registers of other processes. > > Changes since v3: > - Rebase onto tip/master > > Changes since v2: > - Use an exception handler to handle invalid FPU states > (suggested by Andy Lutomirski) > - Check the size of xstate_header.reserved at build time > (suggested by Dave Hansen) > > Eric Biggers (3): > x86/fpu: don't let userspace set bogus xcomp_bv > x86/fpu: tighten validation of user-supplied xstate_header > x86/fpu: reinitialize FPU registers if restoring FPU state fails > > arch/x86/include/asm/fpu/internal.h | 51 +++- > arch/x86/include/asm/fpu/xstate.h | 25 ++ > arch/x86/kernel/fpu/regset.c| 17 +--- > arch/x86/kernel/fpu/signal.c| 18 - > arch/x86/kernel/fpu/xstate.c| 52 > ++--- > arch/x86/mm/extable.c | 24 + > 6 files changed, 102 insertions(+), 85 deletions(-) Thank you Eric - I've applied them and will push it all out together with the other pending bits in WIP.x86/fpu. Ingo
[PATCH] drm/tinydrm: Replace dev_error with DRM_DEV_ERROR
Convert instances of dev_error to DRM_DEV_ERROR as we have
DRM_DEV_ERROR variants of drm print macros.
Signed-off-by: Harsha Sharma
---
drivers/gpu/drm/tinydrm/mi0283qt.c | 8
drivers/gpu/drm/tinydrm/repaper.c | 26 +-
drivers/gpu/drm/tinydrm/st7586.c | 6 +++---
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c
b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7e5bb7d..7dded50 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -31,7 +31,7 @@ static int mi0283qt_init(struct mipi_dbi *mipi)
ret = regulator_enable(mipi->regulator);
if (ret) {
- dev_err(dev, "Failed to enable regulator %d\n", ret);
+ DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret);
return ret;
}
@@ -42,7 +42,7 @@ static int mi0283qt_init(struct mipi_dbi *mipi)
mipi_dbi_hw_reset(mipi);
ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET);
if (ret) {
- dev_err(dev, "Error sending command %d\n", ret);
+ DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
regulator_disable(mipi->regulator);
return ret;
}
@@ -175,13 +175,13 @@ static int mi0283qt_probe(struct spi_device *spi)
mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(mipi->reset)) {
- dev_err(dev, "Failed to get gpio 'reset'\n");
+ DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
return PTR_ERR(mipi->reset);
}
dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
if (IS_ERR(dc)) {
- dev_err(dev, "Failed to get gpio 'dc'\n");
+ DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
return PTR_ERR(dc);
}
diff --git a/drivers/gpu/drm/tinydrm/repaper.c
b/drivers/gpu/drm/tinydrm/repaper.c
index 30dc97b..bd152bb 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -473,7 +473,7 @@ static void repaper_get_temperature(struct repaper_epd *epd)
ret = thermal_zone_get_temp(epd->thermal, &temperature);
if (ret) {
- dev_err(&epd->spi->dev, "Failed to get temperature (%d)\n",
+ DRM_DEV_ERROR(&epd->spi->dev, "Failed to get temperature
(%d)\n",
ret);
return;
}
@@ -629,7 +629,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
mutex_unlock(&tdev->dirty_lock);
if (ret)
- dev_err(fb->dev->dev, "Failed to update display (%d)\n", ret);
+ DRM_DEV_ERROR(fb->dev->dev, "Failed to update display (%d)\n",
ret);
kfree(buf);
return ret;
@@ -703,7 +703,7 @@ static void repaper_pipe_enable(struct
drm_simple_display_pipe *pipe,
}
if (!i) {
- dev_err(dev, "timeout waiting for panel to become ready.\n");
+ DRM_DEV_ERROR(dev, "timeout waiting for panel to become
ready.\n");
power_off(epd);
return;
}
@@ -725,9 +725,9 @@ static void repaper_pipe_enable(struct
drm_simple_display_pipe *pipe,
ret = repaper_read_val(spi, 0x0f);
if (ret < 0 || !(ret & 0x80)) {
if (ret < 0)
- dev_err(dev, "failed to read chip (%d)\n", ret);
+ DRM_DEV_ERROR(dev, "failed to read chip (%d)\n", ret);
else
- dev_err(dev, "panel is reported broken\n");
+ DRM_DEV_ERROR(dev, "panel is reported broken\n");
power_off(epd);
return;
}
@@ -767,7 +767,7 @@ static void repaper_pipe_enable(struct
drm_simple_display_pipe *pipe,
/* check DC/DC */
ret = repaper_read_val(spi, 0x0f);
if (ret < 0) {
- dev_err(dev, "failed to read chip (%d)\n", ret);
+ DRM_DEV_ERROR(dev, "failed to read chip (%d)\n", ret);
power_off(epd);
return;
}
@@ -779,7 +779,7 @@ static void repaper_pipe_enable(struct
drm_simple_display_pipe *pipe,
}
if (!dc_ok) {
- dev_err(dev, "dc/dc failed\n");
+ DRM_DEV_ERROR(dev, "dc/dc failed\n");
power_off(epd);
return;
}
@@ -959,7 +959,7 @@ static int repaper_probe(struct spi_device *spi)
if (IS_ERR(epd->panel_on)) {
ret = PTR_ERR(epd->panel_on);
if (ret != -EPROBE_DEFER)
- dev_err(dev, "Failed to get gpio 'panel-on'\n");
+ DRM_DEV_ERROR(dev, "Failed to get gpio 'panel-on'\n");
return ret;
}
@@ -967,7 +967,7 @@ static int repaper_probe(struct spi_device *spi)
if (IS_ERR(epd->di
Re: [PATCH net-next 2/2] net: dsa: lan9303: Add basic offloading of unicast traffic
Den 22. sep. 2017 22:08, skrev Andrew Lunn: I'm wondering how this is supposed to work. Please add a good comment here, since the hardware is forcing you to do something odd. Maybe it would be a good idea to save the STP state in chip. And then when chip->is_bridged is set true, change the state in the hardware to the saved value? What happens when port 0 is added to the bridge, there is then a minute pause and then port 1 is added? I would expect that as soon as port 0 is added, the STP state machine for port 0 will start and move into listening and then forwarding. Due to hardware limitations it looks like you cannot do this. So what state is the hardware effectively in? Blocking? Forwarding? Then port 1 is added. You can then can respect the states. port 1 will do blocking->listening->forwarding, but what about port 0? The calls won't get repeated? How does it transition to forwarding? Andrew I see your point with the "minute pause" argument. Although a bit contrived use case, it is easy to fix by caching the STP state, as you suggest. So I can do that. I don't think it is contrived. I've done bridge configuration by hand for testing purposes. I've also set the forwarding delay to very small values, so there is a clear race condition here. How does other DSA HW chips handle port separation? Knowing that could perhaps help me know what to look for. They have better hardware :-) Generally each port is totally independent. You can change the STP state per port without restrictions. We can indeed change the STP state per lan9303 port "without restrictions". The point is: Once both external ports are in "forwarding", I see no way to prevent traffic flowing directly between the external ports. Andrew Egil
[PATCH] kfifo: Fix some typo
Fix some typo. Signed-off-by: Christophe JAILLET --- include/linux/kfifo.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/kfifo.h b/include/linux/kfifo.h index 41eb6fdf87a8..7b45959ebd92 100644 --- a/include/linux/kfifo.h +++ b/include/linux/kfifo.h @@ -325,7 +325,7 @@ __kfifo_uint_must_check_helper( \ * * This macro dynamically allocates a new fifo buffer. * - * The numer of elements will be rounded-up to a power of 2. + * The number of elements will be rounded-up to a power of 2. * The fifo will be release with kfifo_free(). * Return 0 if no error, otherwise an error code. */ @@ -358,9 +358,9 @@ __kfifo_int_must_check_helper( \ * @buffer: the preallocated buffer to be used * @size: the size of the internal buffer, this have to be a power of 2 * - * This macro initialize a fifo using a preallocated buffer. + * This macro initializes a fifo using a preallocated buffer. * - * The numer of elements will be rounded-up to a power of 2. + * The number of elements will be rounded-up to a power of 2. * Return 0 if no error, otherwise an error code. */ #define kfifo_init(fifo, buffer, size) \ -- 2.11.0
[PATCH] x86/fpu: Simplify fpu__activate_fpstate_read()
* Eric Biggers wrote:
> On Fri, Sep 22, 2017 at 07:33:14AM +0200, Ingo Molnar wrote:
> >
> > * Eric Biggers wrote:
> >
> > > From: Eric Biggers
> > >
> > > This series fixes the bug found by syzkaller where the ptrace syscall
> > > can be used to set invalid bits in a task's FPU state. I also found
> > > that an equivalent bug was reachable using the sigreturn syscall, so the
> > > first patch fixes the bug in both cases.
> > >
> > > The other two patches start validating the other parts of the
> > > xstate_header and make it so that invalid FPU states can no longer be
> > > abused to leak the FPU registers of other processes.
> > >
> > > Changes since v2:
> > > - Use an exception handler to handle invalid FPU states
> > > (suggested by Andy Lutomirski)
> > > - Check the size of xstate_header.reserved at build time
> > > (suggested by Dave Hansen)
> > >
> > > Eric Biggers (3):
> > > x86/fpu: don't let userspace set bogus xcomp_bv
> > > x86/fpu: tighten validation of user-supplied xstate_header
> > > x86/fpu: reinitialize FPU registers if restoring FPU state fails
> > >
> > > arch/x86/include/asm/fpu/internal.h | 51
> > > +++--
> > > arch/x86/include/asm/fpu/xstate.h | 25 ++
> > > arch/x86/kernel/fpu/regset.c| 20 +++
> > > arch/x86/kernel/fpu/signal.c| 15 ---
> > > arch/x86/kernel/fpu/xstate.c| 27
> > > arch/x86/mm/extable.c | 24 +
> > > 6 files changed, 94 insertions(+), 68 deletions(-)
> >
> > Ok - could you please rebase these to to tip:master that is at:
> >
> > git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
> >
> > In particular this has a WIP.x86/fpu branch with FPU fixes+changes queued
> > up but
> > not merged upstream (yet), which conflict with these changes. I'd like to
> > merge
> > them all together.
> >
>
> Working on it, but there is a problem with current tip. PTRACE_GETREGSET is
> causing the following warning:
Yes, the warning should be harmless, and I fixed it locally earlier today -
does
the patch below solve it for you as well?
Thanks,
Ingo
>
Subject: x86/fpu: Simplify fpu__activate_fpstate_read()
From: Ingo Molnar
Date: Sat Sep 23 11:41:23 CEST 2017
fpu__activate_fpstate_read() can only ever be called for a non-current,
non-executing (stopped) task - so make sure this is checked via a warning
and remove the current-task logic.
This also fixes an incorrect (but harmless) warning triggered by of the earlier
patches.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
Index: linux/arch/x86/kernel/fpu/core.c
===
--- linux.orig/arch/x86/kernel/fpu/core.c
+++ linux/arch/x86/kernel/fpu/core.c
@@ -256,26 +256,18 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*
* If the task has not used the FPU before then initialize its
* fpstate.
- *
- * If the task has used the FPU before then save it.
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- /*
-* If fpregs are active (in the current CPU), then
-* copy them to the fpstate:
-*/
- if (fpu->fpstate_active) {
- fpu__save(fpu);
- } else {
- if (!fpu->fpstate_active) {
- fpstate_init(&fpu->state);
- trace_x86_fpu_init_state(fpu);
+ WARN_ON_FPU(fpu == ¤t->thread.fpu);
+
+ if (!fpu->fpstate_active) {
+ fpstate_init(&fpu->state);
+ trace_x86_fpu_init_state(fpu);
- trace_x86_fpu_activate_state(fpu);
- /* Safe to do for current and for stopped child tasks:
*/
- fpu->fpstate_active = 1;
- }
+ trace_x86_fpu_activate_state(fpu);
+ /* Safe to do for current and for stopped child tasks: */
+ fpu->fpstate_active = 1;
}
}
[PATCH] wireless: iwlwifi: fix minor code style issues
Fixes three trivial issues as reported by checkpatch.pl, namely two switch/case indentation issues and one alignment issue in a multiline comment. Signed-off-by: Christoph Böhmwalder --- drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c index 99676d6c4713..ccdb247d68c5 100644 --- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c +++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c @@ -832,7 +832,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv, capa->standard_phy_calibration_size = le32_to_cpup((__le32 *)tlv_data); break; -case IWL_UCODE_TLV_SEC_RT: + case IWL_UCODE_TLV_SEC_RT: iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR, tlv_len); drv->fw.type = IWL_FW_MVM; @@ -864,7 +864,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv, FW_PHY_CFG_RX_CHAIN) >> FW_PHY_CFG_RX_CHAIN_POS; break; -case IWL_UCODE_TLV_SECURE_SEC_RT: + case IWL_UCODE_TLV_SECURE_SEC_RT: iwl_store_ucode_sec(pieces, tlv_data, IWL_UCODE_REGULAR, tlv_len); drv->fw.type = IWL_FW_MVM; @@ -1335,7 +1335,8 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context) /* Runtime instructions and 2 copies of data: * 1) unmodified from disk -* 2) backup cache for save/restore during power-downs */ +* 2) backup cache for save/restore during power-downs +*/ for (i = 0; i < IWL_UCODE_TYPE_MAX; i++) if (iwl_alloc_ucode(drv, pieces, i)) goto out_free_fw; -- 2.13.5
Re: [PATCH] firmware: dmi_scan: Drop dmi_initialized
* Jean Delvare wrote: > I don't think it makes sense to check for a possible bad > initialization order at run time on every system when it is all > decided at build time. > > A more efficient way to make sure developers do not introduce new > calls to dmi_check_system() too early in the initialization sequence > is to simply document the expected call order. That way, developers > have a chance to get it right immediately, without having to > test-boot their kernel, wonder why it does not work, and parse the > kernel logs for a warning message. And we get rid of the run-time > performance penalty as a nice side effect. Huh? Initialization ordering requirements are very opaque, and by removing the debug check any such bugs are actively hidden. How is documentation supposed to uncover such bugs once they happen? So NAK. Thanks, Ingo
Re: [PATCH v2 0/2] x86: Fix inline asm call constraints for clang
* Josh Poimboeuf wrote: > Patch 1 is a bug fix for an objtool issue which was uncovered by patch 2. > > Patch 2 is the last fix needed for clang to be able to compile and boot > the kernel. > > Josh Poimboeuf (2): > objtool: Handle another GCC stack pointer adjustment bug > x86/asm: Fix inline asm call constraints for clang Have these patches been boot tested with clang? Thanks, Ingo
Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall
On 22/09/2017 14:55, Peter Zijlstra wrote: > You just explained it yourself. If the thread that needs to complete > what you're waiting on has lower priority, it will _never_ get to run if > you're busy waiting on it. > > This is _trivial_. > > And even for !RT it can be quite costly, because you can end up having > to burn your entire slot of CPU time before you run the other task. > > Userspace spinning is _bad_, do not do this. This is not userspace spinning, it is guest spinning---which has effectively the same effect but you cannot quite avoid. But I agree that the solution is properly prioritizing threads that can interrupt the VCPU, and using PI mutexes. I'm not a priori opposed to paravirt scheduling primitives, but I am not at all sure that it's required. Paolo > (the one exception where it works is where you have a single thread per > cpu, because then there's effectively no scheduling).
Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
Hi Greg,
> Phil Elwell hat am 11. August 2017 um 12:20
> geschrieben:
>
>
> The previous commit (0adbfd46) fixed a memory leak but also freed a
> block in the success case, causing a stale pointer to be used with
> potentially fatal results. Only free the vchi_instance block in the
> case that vchi_connect fails; once connected, the instance is
> retained for subsequent connections.
>
> Simplifying the code by removing a bunch of gotos and returning errors
> directly.
>
> Signed-off-by: Phil Elwell
> Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in
> bcm2835_audio_open_connection()")
can you still apply this patch or do you need a resend?
> ---
> [Resend with v2 in subject]
> v2: Simplified following feedback from Dan Carpenter.
> ---
> .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 19
> +++
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> index 5f3d8f2..4be864d 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> @@ -390,8 +390,7 @@ static int bcm2835_audio_open_connection(struct
> bcm2835_alsa_stream *alsa_stream
> __func__, instance);
> instance->alsa_stream = alsa_stream;
> alsa_stream->instance = instance;
> - ret = 0; // xxx todo -1;
> - goto err_free_mem;
> + return 0;
> }
>
> /* Initialize and create a VCHI connection */
> @@ -401,16 +400,15 @@ static int bcm2835_audio_open_connection(struct
> bcm2835_alsa_stream *alsa_stream
> LOG_ERR("%s: failed to initialise VCHI instance
> (ret=%d)\n",
> __func__, ret);
>
> - ret = -EIO;
> - goto err_free_mem;
> + return -EIO;
> }
> ret = vchi_connect(NULL, 0, vchi_instance);
> if (ret) {
> LOG_ERR("%s: failed to connect VCHI instance
> (ret=%d)\n",
> __func__, ret);
>
> - ret = -EIO;
> - goto err_free_mem;
> + kfree(vchi_instance);
> + return -EIO;
> }
> initted = 1;
> }
> @@ -421,19 +419,16 @@ static int bcm2835_audio_open_connection(struct
> bcm2835_alsa_stream *alsa_stream
> if (IS_ERR(instance)) {
> LOG_ERR("%s: failed to initialize audio service\n", __func__);
>
> - ret = PTR_ERR(instance);
> - goto err_free_mem;
> + /* vchi_instance is retained for use the next time. */
> + return PTR_ERR(instance);
> }
>
> instance->alsa_stream = alsa_stream;
> alsa_stream->instance = instance;
>
> LOG_DBG(" success !\n");
> - ret = 0;
> -err_free_mem:
> - kfree(vchi_instance);
>
> - return ret;
> + return 0;
> }
>
> int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream)
> --
> 1.9.1
>
Re: [PATCH] auxdisplay: change PANEL to menuconfig
On Fri, Sep 22, 2017 at 10:20 PM, Randy Dunlap wrote:
> From: Randy Dunlap
>
> This change makes xconfig present the PANEL drivers immediately
> following the AUXDISPLAY drivers instead of under the major menu
> item "Device Drivers". It also unclutters the Device Drivers menu in
> nconfig and menuconfig by moving the PANEL drivers to a sub-menu.
>
> Signed-off-by: Randy Dunlap
> Cc: Andy Shevchenko
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer
On Fri, 22 Sep 2017 23:07:37 -0700 "Paul E. McKenney" wrote: > OK, how about the following? > > It turns out that functions called from rcu_irq_enter() can > be subject to various kinds of tracing, which can result in > rcu_irq_enter() being invoked recursively. This recursion > causes RCU to not have been watching when it should have, > resulting in lockdep-RCU splats. Switching from rcu_irq_enter() > to rcu_nmi_enter() still resulted in failures because of calls > to rcu_irq_enter() between the rcu_nmi_enter() and its matching > rcu_nmi_exit(). Such calls again cause RCU to not be watching > when it should have been. > > In particular, the stack tracer called rcu_irq_enter() > unconditionally, which is problematic when RCU's last > not-watching-to-watching transition was carried out by > rcu_nmi_enter(), as will be the case when tracing uses > rcu_nmi_enter() to cause RCU to start watching the current CPU. > In that case, rcu_irq_enter() actually switches RCU back to > the not-watching state for this CPU, which results in lockdep > splats complaining about rcu_read_lock() being used on an idle > (not-watched) CPU. The first patch of this series addressed > this problem by having rcu_irq_enter() and rcu_irq_exit() > refrain from doing anything when rcu_nmi_enter() caused RCU to > start watching this CPU. The third patch in this series caused > save_stack_trace() to invoke rcu_nmi_enter() and rcu_nmi_exit() > as needed, so this fourth patch now removes the rcu_irq_enter() > and rcu_irq_exit() from within the stack tracer. I think it's a bit too much ;-) I believe it talks too much about the RCU internals, and the bug will be lost on us mere mortals. > > > Actually, thinking about this more, this doesn't need to go in stable. > > As recursive rcu_irq_enter() calls should not hurt, and you now allow > > rcu_irq_enter() to be called even after a rcu_nmi_enter() right? > > Yes, it is now the case that rcu_irq_enter() can be called even after > an rcu_nmi_enter() exited idle, because rcu_irq_enter() now checks for > this and takes an early exit if so. > > But what is it about older kernels prevents the tracing-induced recursive > calls to rcu_irq_enter()? Ah, the bug is if rcu_irq_enter() is called, and the stack trace triggers then. OK, lets keep it simple, and just say this. Currently the stack tracer calls rcu_irq_enter() to make sure RCU is watching when it records a stack trace. But if the stack tracer is triggered while tracing inside of a rcu_irq_enter(), calling rcu_irq_enter() unconditionally can be problematic. The reason for having rcu_irq_enter() in the first place has been fixed from within the saving of the stack trace code, and there's no reason for doing it in the stack tracer itself. Just remove it. Simple and to the point. -- Steve
Re: [PATCH] x86/fpu: Simplify fpu__activate_fpstate_read()
* Ingo Molnar wrote: > > * Eric Biggers wrote: > > > On Fri, Sep 22, 2017 at 07:33:14AM +0200, Ingo Molnar wrote: > > > > > > * Eric Biggers wrote: > > > > > > > From: Eric Biggers > > > > > > > > This series fixes the bug found by syzkaller where the ptrace syscall > > > > can be used to set invalid bits in a task's FPU state. I also found > > > > that an equivalent bug was reachable using the sigreturn syscall, so the > > > > first patch fixes the bug in both cases. > > > > > > > > The other two patches start validating the other parts of the > > > > xstate_header and make it so that invalid FPU states can no longer be > > > > abused to leak the FPU registers of other processes. > > > > > > > > Changes since v2: > > > > - Use an exception handler to handle invalid FPU states > > > > (suggested by Andy Lutomirski) > > > > - Check the size of xstate_header.reserved at build time > > > > (suggested by Dave Hansen) > > > > > > > > Eric Biggers (3): > > > > x86/fpu: don't let userspace set bogus xcomp_bv > > > > x86/fpu: tighten validation of user-supplied xstate_header > > > > x86/fpu: reinitialize FPU registers if restoring FPU state fails > > > > > > > > arch/x86/include/asm/fpu/internal.h | 51 > > > > +++-- > > > > arch/x86/include/asm/fpu/xstate.h | 25 ++ > > > > arch/x86/kernel/fpu/regset.c| 20 +++ > > > > arch/x86/kernel/fpu/signal.c| 15 --- > > > > arch/x86/kernel/fpu/xstate.c| 27 > > > > arch/x86/mm/extable.c | 24 + > > > > 6 files changed, 94 insertions(+), 68 deletions(-) > > > > > > Ok - could you please rebase these to to tip:master that is at: > > > > > > git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master > > > > > > In particular this has a WIP.x86/fpu branch with FPU fixes+changes queued > > > up but > > > not merged upstream (yet), which conflict with these changes. I'd like to > > > merge > > > them all together. > > > > > > > Working on it, but there is a problem with current tip. PTRACE_GETREGSET is > > causing the following warning: > > Yes, the warning should be harmless, and I fixed it locally earlier today - > does > the patch below solve it for you as well? Note that this fix is now part of tip:master as well, so if you re-test -tip you should get all the latest fixes as well (including yours!). Thanks, Ingo
Re: [PATCH v2 0/2] x86: Fix inline asm call constraints for clang
On Sat, Sep 23, 2017 at 12:53:18PM +0200, Ingo Molnar wrote: > > * Josh Poimboeuf wrote: > > > Patch 1 is a bug fix for an objtool issue which was uncovered by patch 2. > > > > Patch 2 is the last fix needed for clang to be able to compile and boot > > the kernel. > > > > Josh Poimboeuf (2): > > objtool: Handle another GCC stack pointer adjustment bug > > x86/asm: Fix inline asm call constraints for clang > > Have these patches been boot tested with clang? Yes, I built and boot them using the directions at: https://github.com/ramosian-glider/clang-kernel-build -- Josh
Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early
On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki wrote:
> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
>> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki wrote:
>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain wrote:
>> >> Ref: https://lkml.org/lkml/2017/9/19/649
>> >>
>> >> The bus controllers should suspend the bus operations only after
>> >> all of the devices on the bus have suspended their device
>> >> completely. Since the i2c_client drivers could be talking to
>> >> their devices in their suspend_late() calls, lets ensure that the
>> >> bus is alive by that time. Thus moving the controller suspend logic to
>> >> suspend_late().
>> >>
>> >> Signed-off-by: Rajat Jain
>> >> ---
>> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> b/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> index 0e65b97842b4..66dd7f844c40 100644
>> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>> >> .prepare = dw_i2c_plat_prepare,
>> >> .complete = dw_i2c_plat_complete,
>> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend,
>> >> dw_i2c_plat_resume)
>> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>> >>dw_i2c_plat_resume,
>> >>NULL)
>> >
>> > No, you can't just do that.
>> >
>> > I sent patches to do it properly before my trip to LA last week, it
>> > shouldn't be overly difficult to find them in the mailing list
>> > archives. I can look them up tomorrow if need be.
>>
>> Thanks, I am guessing you mean this?
>>
>> https://patchwork.kernel.org/patch/9939807/
>
> Yes, that's what I mean.
BTW, does this patchset work for you?
Thanks,
Rafael
[PATCH 00/33] x86 FPU fixes and cleanups for v4.14
So I'd like to push these changes to Linus tomorrow-ish as an RFC pull request in 1-2 days, because there's now 4 fixes depending on these changes, and because the result will be more maintainable for the LTS v4.14 kernel. The biggest changes from the earlier iterations is the fixes from Eric Biggers for information leaks, plus more cleanups. I have also removed the change that Peter Zijlstra and others felt uneasy about, the ::last_cpu -> ::fpregs_cached change to the state machine. This should make the changes uncontroversial. Due to taking out that patch I had to rebase the changes, most of which have accrued months of testing in linux-next. So I'm pretty confident about the overall stability of it. (Famous last words.) Thanks, Ingo ===> Andi Kleen (1): x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU() Eric Biggers (3): x86/fpu: Don't let userspace set bogus xcomp_bv x86/fpu: Tighten validation of user-supplied xstate_header x86/fpu: Reinitialize FPU registers if restoring FPU state fails Ingo Molnar (27): x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user() x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user() x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs x86/fpu: Clean up the parameter definitions of copy_xstate_to_*() x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*() x86/fpu: Simplify __copy_xstate_to_kernel() return values x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate() x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API x86/fpu: Flip the parameter order in copy_*_to_xstate() x86/fpu: Simplify fpu->fpregs_active use x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic x86/fpu: Split the state handling in fpu__drop() x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active x86/fpu: Remove struct fpu::fpregs_active x86/fpu: Simplify fpu__activate_fpstate_read() x86/fpu: Remove fpu__current_fpstate_write_begin/end() x86/fpu: Rename fpu::fpstate_active to fpu::initialized x86/fpu: Fix stale comments about lazy FPU logic x86/fpu: Simplify and speed up fpu__copy() x86/fpu: Rename fpu__activate_curr() to fpu__initialize() x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__read/write() Rik van Riel (1): x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs kbuild test robot (1): x86/fpu: Fix boolreturn.cocci warnings arch/x86/ia32/ia32_signal.c | 2 +- arch/x86/include/asm/fpu/internal.h | 90 ++--- arch/x86/include/asm/fpu/types.h| 32 ++-- arch/x86/include/asm/fpu/xstate.h | 33 +++- arch/x86/include/asm/trace/fpu.h| 11 +-- arch/x86/kernel/fpu/core.c | 158 + arch/x86/kernel/fpu/init.c | 2 +- arch/x86/kernel/fpu/regset.c| 48 ++-- arch/x86/kernel/fpu/signal.c| 36 + arch/x86/kernel/fpu/xstate.c| 240 arch/x86/kernel/signal.c| 6 +- arch/x86/kvm/x86.c | 2 +- arch/x86/math-emu/fpu_entry.c | 2 +- arch/x86/mm/extable.c | 24 ++ arch/x86/mm/pkeys.c | 3 +- 15 files changed, 367 insertions(+), 322 deletions(-) -- 2.11.0
[PATCH 01/33] x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user()
The 'copyin/copyout' nomenclature needlessly departs from what the modern FPU
code
uses, which is:
copy_fpregs_to_fpstate()
copy_fpstate_to_sigframe()
copy_fregs_to_user()
copy_fxregs_to_kernel()
copy_fxregs_to_user()
copy_kernel_to_fpregs()
copy_kernel_to_fregs()
copy_kernel_to_fxregs()
copy_kernel_to_xregs()
copy_user_to_fregs()
copy_user_to_fxregs()
copy_user_to_xregs()
copy_xregs_to_kernel()
copy_xregs_to_user()
I.e. according to this pattern, the following rename should be done:
copyin_to_xsaves()-> copy_user_to_xstate()
copyout_from_xsaves() -> copy_xstate_to_user()
or, if we want to be pedantic, denote that that the user-space format is ptrace:
copyin_to_xsaves()-> copy_user_ptrace_to_xstate()
copyout_from_xsaves() -> copy_xstate_to_user_ptrace()
But I'd suggest the shorter, non-pedantic name.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 4 ++--
arch/x86/kernel/fpu/signal.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 1b2799e0699a..a1baa17e9748 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
void __user *ubuf, struct xregs_state *xsave);
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
+int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b188b16841e3..165d0545c924 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -92,7 +92,7 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
fpu__activate_fpstate_read(fpu);
if (using_compacted_format()) {
- ret = copyout_from_xsaves(pos, count, kbuf, ubuf, xsave);
+ ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
} else {
fpstate_sanitize_xstate(fpu);
/*
@@ -132,7 +132,7 @@ int xstateregs_set(struct task_struct *target, const struct
user_regset *regset,
fpu__activate_fpstate_write(fpu);
if (boot_cpu_has(X86_FEATURE_XSAVES))
- ret = copyin_to_xsaves(kbuf, ubuf, xsave);
+ ret = copy_user_to_xstate(kbuf, ubuf, xsave);
else
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0,
-1);
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 83c23c230b4c..b1fe9a1fc4e0 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -324,7 +324,7 @@ static int __fpu__restore_sig(void __user *buf, void __user
*buf_fx, int size)
fpu__drop(fpu);
if (using_compacted_format()) {
- err = copyin_to_xsaves(NULL, buf_fx,
+ err = copy_user_to_xstate(NULL, buf_fx,
&fpu->state.xsave);
} else {
err = __copy_from_user(&fpu->state.xsave,
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c24ac1efb12d..e7bb41723eaa 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -951,7 +951,7 @@ static inline int xstate_copyout(unsigned int pos, unsigned
int count,
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
void __user *ubuf, struct xregs_state *xsave)
{
unsigned int offset, size;
@@ -1023,7 +1023,7 @@ int copyout_from_xsaves(unsigned int pos, unsigned int
count, void *kbuf,
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
*/
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
+int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave)
{
unsigned int offset, size;
--
2.11.0
[PATCH 04/33] x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
The 'kbuf' parameter is unused in the _user() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 25 +++--
3 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index c762574a245f..65bd68c30cd0 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -49,7 +49,7 @@ void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void
__user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user
*ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 34e74adf9d5d..fd6dbdd8fde6 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -95,7 +95,7 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
if (kbuf)
ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
- ret = copy_xstate_to_user(pos, count, kbuf, ubuf,
xsave);
+ ret = copy_xstate_to_user(pos, count, ubuf, xsave);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 71d3bda2b898..2d8f3344875d 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1011,10 +1011,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf, stru
}
static inline int
-__copy_xstate_to_user(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_user(unsigned int pos, unsigned int count, void __user *ubuf,
const void *data, const int start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -1022,12 +1019,8 @@ __copy_xstate_to_user(unsigned int pos, unsigned int
count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos -
pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ if (__copy_to_user(ubuf + pos, data, copy))
+ return -EFAULT;
}
return 0;
}
@@ -1038,8 +1031,7 @@ __copy_xstate_to_user(unsigned int pos, unsigned int
count,
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user
*ubuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -1064,8 +1056,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int
count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf, &header, 0,
count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, &header, 0, count);
if (ret)
return ret;
@@ -1079,8 +1070,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int
count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(offset, size, kbuf, ubuf,
src, 0, count);
-
+ ret = __copy_xstate_to_user(offset, size, ubuf, src, 0,
count);
if (ret)
return ret;
@@ -1096,8 +1086,7 @@ int copy_xstate_to_user(unsigned int pos, unsigned int
count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(offset, si
[PATCH 27/33] x86/fpu: Simplify fpu__activate_fpstate_read()
fpu__activate_fpstate_read() can only ever be called for a non-current,
non-executing (stopped) task - so make sure this is checked via a warning
and remove the current-task logic.
This also fixes an incorrect (but harmless) warning introduced by one of
the earlier patches.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 93103a909c47..7bd4edb76c1d 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -256,26 +256,18 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*
* If the task has not used the FPU before then initialize its
* fpstate.
- *
- * If the task has used the FPU before then save it.
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
- /*
-* If fpregs are active (in the current CPU), then
-* copy them to the fpstate:
-*/
- if (fpu->fpstate_active) {
- fpu__save(fpu);
- } else {
- if (!fpu->fpstate_active) {
- fpstate_init(&fpu->state);
- trace_x86_fpu_init_state(fpu);
+ WARN_ON_FPU(fpu == ¤t->thread.fpu);
- trace_x86_fpu_activate_state(fpu);
- /* Safe to do for current and for stopped child tasks:
*/
- fpu->fpstate_active = 1;
- }
+ if (!fpu->fpstate_active) {
+ fpstate_init(&fpu->state);
+ trace_x86_fpu_init_state(fpu);
+
+ trace_x86_fpu_activate_state(fpu);
+ /* Safe to do for current and for stopped child tasks: */
+ fpu->fpstate_active = 1;
}
}
--
2.11.0
[PATCH 32/33] x86/fpu: Rename fpu__activate_curr() to fpu__initialize()
Rename this function to better express that it's all about
initializing the FPU state of a task which goes hand in hand
with the fpu::initialized field.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 2 +-
arch/x86/kernel/fpu/core.c | 8
arch/x86/kernel/fpu/signal.c| 2 +-
arch/x86/kvm/x86.c | 2 +-
arch/x86/math-emu/fpu_entry.c | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index b26ae05da18a..7c980aafb8aa 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -23,7 +23,7 @@
/*
* High level FPU state handling functions:
*/
-extern void fpu__activate_curr(struct fpu *fpu);
+extern void fpu__initialize(struct fpu *fpu);
extern void fpu__activate_fpstate_read(struct fpu *fpu);
extern void fpu__activate_fpstate_write(struct fpu *fpu);
extern void fpu__save(struct fpu *fpu);
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index e935f8faf287..ca580d90799d 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -224,7 +224,7 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
* Activate the current task's in-memory FPU context,
* if it has not been used before:
*/
-void fpu__activate_curr(struct fpu *fpu)
+void fpu__initialize(struct fpu *fpu)
{
WARN_ON_FPU(fpu != ¤t->thread.fpu);
@@ -237,7 +237,7 @@ void fpu__activate_curr(struct fpu *fpu)
fpu->initialized = 1;
}
}
-EXPORT_SYMBOL_GPL(fpu__activate_curr);
+EXPORT_SYMBOL_GPL(fpu__initialize);
/*
* This function must be called before we read a task's fpstate.
@@ -305,7 +305,7 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
*/
void fpu__restore(struct fpu *fpu)
{
- fpu__activate_curr(fpu);
+ fpu__initialize(fpu);
/* Avoid __kernel_fpu_begin() right after fpregs_activate() */
kernel_fpu_disable();
@@ -381,7 +381,7 @@ void fpu__clear(struct fpu *fpu)
*/
if (static_cpu_has(X86_FEATURE_FPU)) {
preempt_disable();
- fpu__activate_curr(fpu);
+ fpu__initialize(fpu);
user_fpu_begin();
copy_init_fpstate_to_fpregs();
preempt_enable();
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 34c9a85eb5ba..afe54247cf27 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -283,7 +283,7 @@ static int __fpu__restore_sig(void __user *buf, void __user
*buf_fx, int size)
if (!access_ok(VERIFY_READ, buf, size))
return -EACCES;
- fpu__activate_curr(fpu);
+ fpu__initialize(fpu);
if (!static_cpu_has(X86_FEATURE_FPU))
return fpregs_soft_set(current, NULL,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd17b7d9a107..03869eb7fcd6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7225,7 +7225,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct
kvm_run *kvm_run)
int r;
sigset_t sigsaved;
- fpu__activate_curr(fpu);
+ fpu__initialize(fpu);
if (vcpu->sigset_active)
sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index d4a7df2205b8..220638a4cb94 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -114,7 +114,7 @@ void math_emulate(struct math_emu_info *info)
struct desc_struct code_descriptor;
struct fpu *fpu = ¤t->thread.fpu;
- fpu__activate_curr(fpu);
+ fpu__initialize(fpu);
#ifdef RE_ENTRANT_CHECKING
if (emulating) {
--
2.11.0
[PATCH 23/33] x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU()
From: Andi Kleen copy_xregs_to_kernel checks if the alternatives have been already patched. This WARN_ON() is always executed in every context switch. All the other checks in fpu internal.h are WARN_ON_FPU(), but this one is plain WARN_ON(). I assume it was forgotten to switch it. So switch it to WARN_ON_FPU() too to avoid some unnecessary code in the context switch, and a potentially expensive cache line miss for the global variable. Signed-off-by: Andi Kleen Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/20170329062605.4970-1-a...@firstfloor.org Signed-off-by: Ingo Molnar --- arch/x86/include/asm/fpu/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 629e7abcd6c9..2dca7c65319c 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -350,7 +350,7 @@ static inline void copy_xregs_to_kernel(struct xregs_state *xstate) u32 hmask = mask >> 32; int err; - WARN_ON(!alternatives_patched); + WARN_ON_FPU(!alternatives_patched); XSTATE_XSAVE(xstate, lmask, hmask, err); -- 2.11.0
[PATCH 33/33] x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__read/write()
As per the new nomenclature we don't 'activate' the FPU state
anymore, we initialize it. So drop the _activate_fpstate name
from these functions, which were a bit of a mouthful anyway.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 4 ++--
arch/x86/kernel/fpu/core.c | 4 ++--
arch/x86/kernel/fpu/regset.c| 12 ++--
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 7c980aafb8aa..8d90b8afc7a8 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -24,8 +24,8 @@
* High level FPU state handling functions:
*/
extern void fpu__initialize(struct fpu *fpu);
-extern void fpu__activate_fpstate_read(struct fpu *fpu);
-extern void fpu__activate_fpstate_write(struct fpu *fpu);
+extern void fpu__read(struct fpu *fpu);
+extern void fpu__write(struct fpu *fpu);
extern void fpu__save(struct fpu *fpu);
extern void fpu__restore(struct fpu *fpu);
extern int fpu__restore_sig(void __user *buf, int ia32_frame);
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index ca580d90799d..99033eeb7ec0 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -245,7 +245,7 @@ EXPORT_SYMBOL_GPL(fpu__initialize);
* If the task has not used the FPU before then initialize its
* fpstate.
*/
-void fpu__activate_fpstate_read(struct fpu *fpu)
+void fpu__read(struct fpu *fpu)
{
WARN_ON_FPU(fpu == ¤t->thread.fpu);
@@ -272,7 +272,7 @@ void fpu__activate_fpstate_read(struct fpu *fpu)
* state pending on its former CPU could be restored, corrupting
* the modifications.
*/
-void fpu__activate_fpstate_write(struct fpu *fpu)
+void fpu__write(struct fpu *fpu)
{
/*
* Only stopped child tasks can be used to modify the FPU
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 5c2a051baeac..2bbcc3d09bb5 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -38,7 +38,7 @@ int xfpregs_get(struct task_struct *target, const struct
user_regset *regset,
if (!boot_cpu_has(X86_FEATURE_FXSR))
return -ENODEV;
- fpu__activate_fpstate_read(fpu);
+ fpu__read(fpu);
fpstate_sanitize_xstate(fpu);
return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
@@ -55,7 +55,7 @@ int xfpregs_set(struct task_struct *target, const struct
user_regset *regset,
if (!boot_cpu_has(X86_FEATURE_FXSR))
return -ENODEV;
- fpu__activate_fpstate_write(fpu);
+ fpu__write(fpu);
fpstate_sanitize_xstate(fpu);
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
@@ -89,7 +89,7 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
xsave = &fpu->state.xsave;
- fpu__activate_fpstate_read(fpu);
+ fpu__read(fpu);
if (using_compacted_format()) {
if (kbuf)
@@ -132,7 +132,7 @@ int xstateregs_set(struct task_struct *target, const struct
user_regset *regset,
xsave = &fpu->state.xsave;
- fpu__activate_fpstate_write(fpu);
+ fpu__write(fpu);
if (using_compacted_format()) {
if (kbuf)
@@ -303,7 +303,7 @@ int fpregs_get(struct task_struct *target, const struct
user_regset *regset,
struct fpu *fpu = &target->thread.fpu;
struct user_i387_ia32_struct env;
- fpu__activate_fpstate_read(fpu);
+ fpu__read(fpu);
if (!boot_cpu_has(X86_FEATURE_FPU))
return fpregs_soft_get(target, regset, pos, count, kbuf, ubuf);
@@ -333,7 +333,7 @@ int fpregs_set(struct task_struct *target, const struct
user_regset *regset,
struct user_i387_ia32_struct env;
int ret;
- fpu__activate_fpstate_write(fpu);
+ fpu__write(fpu);
fpstate_sanitize_xstate(fpu);
if (!boot_cpu_has(X86_FEATURE_FPU))
--
2.11.0
[PATCH 31/33] x86/fpu: Simplify and speed up fpu__copy()
fpu__copy() has a preempt_disable()/enable() pair, which it had to do to
be able to atomically unlazy the current task when doing an FNSAVE.
But we don't unlazy tasks anymore, we always do direct saves/restores of
FPU context.
So remove both the unnecessary critical section, and update the comments.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 4acfc0ebc160..e935f8faf287 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -206,22 +206,13 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
* Save current FPU registers directly into the child
* FPU context, without any memory-to-memory copying.
*
-* We have to do all this with preemption disabled,
-* mostly because of the FNSAVE case, because in that
-* case we must not allow preemption in the window
-* between the FNSAVE and us marking the context lazy.
-*
-* It shouldn't be an issue as even FNSAVE is plenty
-* fast in terms of critical section length.
+* ( The function 'fails' in the FNSAVE case, which destroys
+* register contents so we have to copy them back. )
*/
- preempt_disable();
if (!copy_fpregs_to_fpstate(dst_fpu)) {
- memcpy(&src_fpu->state, &dst_fpu->state,
- fpu_kernel_xstate_size);
-
+ memcpy(&src_fpu->state, &dst_fpu->state,
fpu_kernel_xstate_size);
copy_kernel_to_fpregs(&src_fpu->state);
}
- preempt_enable();
trace_x86_fpu_copy_src(src_fpu);
trace_x86_fpu_copy_dst(dst_fpu);
--
2.11.0
[PATCH 26/33] x86/fpu: Reinitialize FPU registers if restoring FPU state fails
From: Eric Biggers
Userspace can change the FPU state of a task using the ptrace() or
rt_sigreturn() system calls. Because reserved bits in the FPU state can
cause the XRSTOR instruction to fail, the kernel has to carefully
validate that no reserved bits or other invalid values are being set.
Unfortunately, there have been bugs in this validation code. For
example, we were not checking that the 'xcomp_bv' field in the
xstate_header was 0. As-is, such bugs are exploitable to read the FPU
registers of other processes on the system. To do so, an attacker can
create a task, assign to it an invalid FPU state, then spin in a loop
and monitor the values of the FPU registers. Because the task's FPU
registers are not being restored, sometimes the FPU registers will have
the values from another process.
This is likely to continue to be a problem in the future because the
validation done by the CPU instructions like XRSTOR is not immediately
visible to kernel developers. Nor will invalid FPU states ever be
encountered during ordinary use --- they will only be seen during
fuzzing or exploits. There can even be reserved bits outside the
xstate_header which are easy to forget about. For example, the MXCSR
register contains reserved bits, which were not validated by the
KVM_SET_XSAVE ioctl until commit a575813bfe4b ("KVM: x86: Fix load
damaged SSEx MXCSR register").
Therefore, mitigate this class of vulnerability by restoring the FPU
registers from init_fpstate if restoring from the task's state fails.
We actually used to do this, but it was (perhaps unwisely) removed by
commit 9ccc27a5d297 ("x86/fpu: Remove error return values from
copy_kernel_to_*regs() functions"). This new patch is also a bit
different. First, it only clears the registers, not also the bad
in-memory state; this is simpler and makes it easier to make the
mitigation cover all callers of __copy_kernel_to_fpregs(). Second, it
does the register clearing in an exception handler so that no extra
instructions are added to context switches. In fact, we *remove*
instructions, since previously we were always zeroing the register
containing 'err' even if CONFIG_X86_DEBUG_FPU was disabled.
Signed-off-by: Eric Biggers
Reviewed-by: Rik van Riel
Cc: Andy Lutomirski
Cc: Dave Hansen
Cc: Dmitry Vyukov
Cc: Fenghua Yu
Cc: Kevin Hao
Cc: Linus Torvalds
Cc: Michael Halcrow
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Wanpeng Li
Cc: Yu-cheng Yu
Cc: kernel-harden...@lists.openwall.com
Link: http://lkml.kernel.org/r/20170922174156.16780-4-ebigge...@gmail.com
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 51
+++
arch/x86/mm/extable.c | 24
2 files changed, 39 insertions(+), 36 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 2dca7c65319c..cf290d424e48 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -120,20 +120,11 @@ extern void fpstate_sanitize_xstate(struct fpu *fpu);
err;\
})
-#define check_insn(insn, output, input...) \
-({ \
- int err;\
+#define kernel_insn(insn, output, input...)\
asm volatile("1:" #insn "\n\t" \
"2:\n" \
-".section .fixup,\"ax\"\n" \
-"3: movl $-1,%[err]\n"\
-"jmp 2b\n"\
-".previous\n" \
-_ASM_EXTABLE(1b, 3b) \
-: [err] "=r" (err), output \
-: "0"(0), input); \
- err;\
-})
+_ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_fprestore) \
+: output : input)
static inline int copy_fregs_to_user(struct fregs_state __user *fx)
{
@@ -153,20 +144,16 @@ static inline int copy_fxregs_to_user(struct fxregs_state
__user *fx)
static inline void copy_kernel_to_fxregs(struct fxregs_state *fx)
{
- int err;
-
if (IS_ENABLED(CONFIG_X86_32)) {
- err = check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
+ kernel_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
} else {
if (IS_ENABLED(CONFIG_AS_FXSAVEQ)) {
- err = check_insn(fxrstorq %[fx], "=m" (*fx), [fx] "m"
(*fx));
+ kernel_insn(fxrstorq
[PATCH 30/33] x86/fpu: Fix stale comments about lazy FPU logic
We don't do any lazy restore anymore, what we have are two pieces of
optimization:
- no-FPU tasks that don't save/restore the FPU context (kernel threads are
such)
- cached FPU registers maintained via the fpu->last_cpu field. This means that
if an FPU task context switches to a non-FPU task then we can maintain the
FPU registers as an in-FPU copies (cache), and skip the restoration of them
once we switch back to the original FPU-using task.
Update all the comments that still referred to old 'lazy' and 'unlazy' concepts.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index d770f9a6d4e1..4acfc0ebc160 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -205,9 +205,6 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
/*
* Save current FPU registers directly into the child
* FPU context, without any memory-to-memory copying.
-* In lazy mode, if the FPU context isn't loaded into
-* fpregs, CR0.TS will be set and do_device_not_available
-* will load the FPU context.
*
* We have to do all this with preemption disabled,
* mostly because of the FNSAVE case, because in that
@@ -274,13 +271,13 @@ void fpu__activate_fpstate_read(struct fpu *fpu)
/*
* This function must be called before we write a task's fpstate.
*
- * If the task has used the FPU before then unlazy it.
+ * If the task has used the FPU before then invalidate any cached FPU
registers.
* If the task has not used the FPU before then initialize its fpstate.
*
* After this function call, after registers in the fpstate are
* modified and the child task has woken up, the child task will
* restore the modified FPU state from the modified context. If we
- * didn't clear its lazy status here then the lazy in-registers
+ * didn't clear its cached status here then the cached in-registers
* state pending on its former CPU could be restored, corrupting
* the modifications.
*/
@@ -293,7 +290,7 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
WARN_ON_FPU(fpu == ¤t->thread.fpu);
if (fpu->initialized) {
- /* Invalidate any lazy state: */
+ /* Invalidate any cached state: */
__fpu_invalidate_fpregs_state(fpu);
} else {
fpstate_init(&fpu->state);
--
2.11.0
[PATCH 29/33] x86/fpu: Rename fpu::fpstate_active to fpu::initialized
The x86 FPU code used to have a complex state machine where both the FPU
registers and the FPU state context could be 'active' (or inactive)
independently of each other - which enabled features like lazy FPU restore.
Much of this complexity is gone in the current code: now we basically can
have FPU-less tasks (kernel threads) that don't use (and save/restore) FPU
state at all, plus full FPU users that save/restore directly with no laziness
whatsoever.
But the fpu::fpstate_active still carries bits of the old complexity - meanwhile
this flag has become a simple flag that shows whether the FPU context saving
area in the thread struct is initialized and used, or not.
Rename it to fpu::initialized to express this simplicity in the name as well.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/ia32/ia32_signal.c | 2 +-
arch/x86/include/asm/fpu/internal.h | 4 ++--
arch/x86/include/asm/fpu/types.h| 6 +++---
arch/x86/include/asm/trace/fpu.h| 8
arch/x86/kernel/fpu/core.c | 24
arch/x86/kernel/fpu/init.c | 2 +-
arch/x86/kernel/fpu/regset.c| 6 +++---
arch/x86/kernel/fpu/signal.c| 8
arch/x86/kernel/fpu/xstate.c| 2 +-
arch/x86/kernel/signal.c| 6 +++---
arch/x86/mm/pkeys.c | 2 +-
11 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index e0bb46c02857..0e2a5edbce00 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -231,7 +231,7 @@ static void __user *get_sigframe(struct ksignal *ksig,
struct pt_regs *regs,
ksig->ka.sa.sa_restorer)
sp = (unsigned long) ksig->ka.sa.sa_restorer;
- if (fpu->fpstate_active) {
+ if (fpu->initialized) {
unsigned long fx_aligned, math_size;
sp = fpu__alloc_mathframe(sp, 1, &fx_aligned, &math_size);
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 508e4181c4af..b26ae05da18a 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -527,7 +527,7 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- if (old_fpu->fpstate_active) {
+ if (old_fpu->initialized) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
else
@@ -550,7 +550,7 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
static inline void switch_fpu_finish(struct fpu *new_fpu, int cpu)
{
bool preload = static_cpu_has(X86_FEATURE_FPU) &&
- new_fpu->fpstate_active;
+ new_fpu->initialized;
if (preload) {
if (!fpregs_state_valid(new_fpu, cpu))
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 71db45ca8870..a1520575d86b 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -293,13 +293,13 @@ struct fpu {
unsigned intlast_cpu;
/*
-* @fpstate_active:
+* @initialized:
*
-* This flag indicates whether this context is active: if the task
+* This flag indicates whether this context is initialized: if the task
* is not running then we can restore from this context, if the task
* is running then we should save into this context.
*/
- unsigned char fpstate_active;
+ unsigned char initialized;
/*
* @state:
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index da565aae9fd2..39f7a27bef13 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,22 +12,22 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_STRUCT__entry(
__field(struct fpu *, fpu)
- __field(bool, fpstate_active)
+ __field(bool, initialized)
__field(u64, xfeatures)
__field(u64, xcomp_bv)
),
TP_fast_assign(
__entry->fpu= fpu;
- __entry->fpstate_active = fpu->fpstate_active;
+ __entry->initialized= fpu->initialized;
if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
__entry->xfeatures = fpu->state.xsave.header.xfeatures;
__entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv;
}
),
- TP_printk("x86/fpu: %p fpstate_active: %d xfeatures: %llx xcomp_bv:
%llx",
+ TP_printk("x86/fpu: %p initialized: %d xfeatures: %llx xcomp_bv: %llx"
[PATCH 28/33] x86/fpu: Remove fpu__current_fpstate_write_begin/end()
These functions are not used anymore, so remove them.
Cc: Andy Lutomirski
Cc: Bobby Powers
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Eric Biggers
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 2 --
arch/x86/kernel/fpu/core.c | 63
-
2 files changed, 65 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index cf290d424e48..508e4181c4af 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -26,8 +26,6 @@
extern void fpu__activate_curr(struct fpu *fpu);
extern void fpu__activate_fpstate_read(struct fpu *fpu);
extern void fpu__activate_fpstate_write(struct fpu *fpu);
-extern void fpu__current_fpstate_write_begin(void);
-extern void fpu__current_fpstate_write_end(void);
extern void fpu__save(struct fpu *fpu);
extern void fpu__restore(struct fpu *fpu);
extern int fpu__restore_sig(void __user *buf, int ia32_frame);
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 7bd4edb76c1d..c248ce5ceff4 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -306,69 +306,6 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
}
/*
- * This function must be called before we write the current
- * task's fpstate.
- *
- * This call gets the current FPU register state and moves
- * it in to the 'fpstate'. Preemption is disabled so that
- * no writes to the 'fpstate' can occur from context
- * swiches.
- *
- * Must be followed by a fpu__current_fpstate_write_end().
- */
-void fpu__current_fpstate_write_begin(void)
-{
- struct fpu *fpu = ¤t->thread.fpu;
-
- /*
-* Ensure that the context-switching code does not write
-* over the fpstate while we are doing our update.
-*/
- preempt_disable();
-
- /*
-* Move the fpregs in to the fpu's 'fpstate'.
-*/
- fpu__activate_fpstate_read(fpu);
-
- /*
-* The caller is about to write to 'fpu'. Ensure that no
-* CPU thinks that its fpregs match the fpstate. This
-* ensures we will not be lazy and skip a XRSTOR in the
-* future.
-*/
- __fpu_invalidate_fpregs_state(fpu);
-}
-
-/*
- * This function must be paired with fpu__current_fpstate_write_begin()
- *
- * This will ensure that the modified fpstate gets placed back in
- * the fpregs if necessary.
- *
- * Note: This function may be called whether or not an _actual_
- * write to the fpstate occurred.
- */
-void fpu__current_fpstate_write_end(void)
-{
- struct fpu *fpu = ¤t->thread.fpu;
-
- /*
-* 'fpu' now has an updated copy of the state, but the
-* registers may still be out of date. Update them with
-* an XRSTOR if they are active.
-*/
- if (fpu->fpstate_active)
- copy_kernel_to_fpregs(&fpu->state);
-
- /*
-* Our update is done and the fpregs/fpstate are in sync
-* if necessary. Context switches can happen again.
-*/
- preempt_enable();
-}
-
-/*
* 'fpu__restore()' is called to copy FPU registers from
* the FPU fpstate to the live hw registers and to activate
* access to the hardware registers, so that FPU instructions
--
2.11.0
[PATCH 25/33] x86/fpu: Tighten validation of user-supplied xstate_header
From: Eric Biggers
Move validation of user-supplied xstate_headers into a helper function
and call it from both the ptrace and sigreturn syscall paths. The new
function also considers it to be an error if *any* reserved bits are
set, whereas before we were just clearing most of them.
This should reduce the chance of bugs that fail to correctly validate
user-supplied XSAVE areas. It also will expose any broken userspace
programs that set the other reserved bits; this is desirable because
such programs will lose compatibility with future CPUs and kernels if
those bits are ever used for anything. (There shouldn't be any such
programs, and in fact in the case where the compacted format is in use
we were already validating xfeatures. But you never know...)
Signed-off-by: Eric Biggers
Reviewed-by: Kees Cook
Reviewed-by: Rik van Riel
Acked-by: Dave Hansen
Cc: Andy Lutomirski
Cc: Dmitry Vyukov
Cc: Fenghua Yu
Cc: Kevin Hao
Cc: Linus Torvalds
Cc: Michael Halcrow
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Wanpeng Li
Cc: Yu-cheng Yu
Cc: kernel-harden...@lists.openwall.com
Link: http://lkml.kernel.org/r/20170922174156.16780-3-ebigge...@gmail.com
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 25 +
arch/x86/kernel/fpu/regset.c | 21 +++--
arch/x86/kernel/fpu/signal.c | 17 +
arch/x86/kernel/fpu/xstate.c | 52
+++-
4 files changed, 60 insertions(+), 55 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 579ac2358e63..3d79d0ee4d30 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -52,4 +52,29 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
+
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+static inline int validate_xstate_header(const struct xstate_header *hdr)
+{
+ /* No unknown or supervisor features may be set */
+ if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+ return -EINVAL;
+
+ /* Userspace must use the uncompacted format */
+ if (hdr->xcomp_bv)
+ return -EINVAL;
+
+ /*
+* If 'reserved' is shrunken to add a new field, make sure to validate
+* that new field here!
+*/
+ BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+ /* No reserved bits may be set */
+ if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+ return -EINVAL;
+
+ return 0;
+}
+
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index c764f7405322..0467e536b0a2 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -134,34 +134,27 @@ int xstateregs_set(struct task_struct *target, const
struct user_regset *regset,
fpu__activate_fpstate_write(fpu);
- if (boot_cpu_has(X86_FEATURE_XSAVES)) {
+ if (using_compacted_format()) {
if (kbuf)
ret = copy_kernel_to_xstate(xsave, kbuf);
else
ret = copy_user_to_xstate(xsave, ubuf);
} else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0,
-1);
-
- /* xcomp_bv must be 0 when using uncompacted format */
- if (!ret && xsave->header.xcomp_bv)
- ret = -EINVAL;
+ if (!ret)
+ ret = validate_xstate_header(&xsave->header);
}
/*
-* In case of failure, mark all states as init:
-*/
- if (ret)
- fpstate_init(&fpu->state);
-
- /*
* mxcsr reserved bits must be masked to zero for security reasons.
*/
xsave->i387.mxcsr &= mxcsr_feature_mask;
- xsave->header.xfeatures &= xfeatures_mask;
+
/*
-* These bits must be zero.
+* In case of failure, mark all states as init:
*/
- memset(&xsave->header.reserved, 0, 48);
+ if (ret)
+ fpstate_init(&fpu->state);
return ret;
}
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index d34349934702..5b5d75e3b2a4 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -214,8 +214,11 @@ sanitize_restored_xstate(struct task_struct *tsk,
struct xstate_header *header = &xsave->header;
if (use_xsave()) {
- /* These bits must be zero. */
- memset(header->reserved, 0, 48);
+ /*
+* Note: we don't need to zero the reserv
Re: [PATCH 00/33] x86 FPU fixes and cleanups for v4.14
* Ingo Molnar wrote: > So I'd like to push these changes to Linus tomorrow-ish as an RFC pull > request in 1-2 days, because there's now 4 fixes depending on these > changes, and because the result will be more maintainable for the > LTS v4.14 kernel. > > The biggest changes from the earlier iterations is the fixes from > Eric Biggers for information leaks, plus more cleanups. I have also > removed the change that Peter Zijlstra and others felt uneasy about, > the ::last_cpu -> ::fpregs_cached change to the state machine. This > should make the changes uncontroversial. > > Due to taking out that patch I had to rebase the changes, most of which > have accrued months of testing in linux-next. So I'm pretty confident > about the overall stability of it. (Famous last words.) I forgot to mention that these changes are also available in tip:master (and tip:WIP.x86/fpu), at: git git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git master Thanks, Ingo
[PATCH 20/33] x86/fpu: Remove struct fpu::fpregs_active
The previous changes paved the way for the removal of the
fpu::fpregs_active state flag - we now only have the
fpu::fpstate_active and fpu::last_cpu fields left.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 5 -
arch/x86/include/asm/fpu/types.h| 23 ---
arch/x86/include/asm/trace/fpu.h| 5 +
arch/x86/kernel/fpu/core.c | 9 -
arch/x86/kernel/fpu/signal.c| 2 --
5 files changed, 1 insertion(+), 43 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 42a601673c09..629e7abcd6c9 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -526,14 +526,12 @@ static inline int fpregs_state_valid(struct fpu *fpu,
unsigned int cpu)
*/
static inline void fpregs_deactivate(struct fpu *fpu)
{
- fpu->fpregs_active = 0;
this_cpu_write(fpu_fpregs_owner_ctx, NULL);
trace_x86_fpu_regs_deactivated(fpu);
}
static inline void fpregs_activate(struct fpu *fpu)
{
- fpu->fpregs_active = 1;
this_cpu_write(fpu_fpregs_owner_ctx, fpu);
trace_x86_fpu_regs_activated(fpu);
}
@@ -552,8 +550,6 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
-
if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
@@ -561,7 +557,6 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
old_fpu->last_cpu = cpu;
/* But leave fpu_fpregs_owner_ctx! */
- old_fpu->fpregs_active = 0;
trace_x86_fpu_regs_deactivated(old_fpu);
} else
old_fpu->last_cpu = -1;
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 3c80f5b9c09d..0c314a397cf5 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -299,29 +299,6 @@ struct fpu {
unsigned char fpstate_active;
/*
-* @fpregs_active:
-*
-* This flag determines whether a given context is actively
-* loaded into the FPU's registers and that those registers
-* represent the task's current FPU state.
-*
-* Note the interaction with fpstate_active:
-*
-* # task does not use the FPU:
-* fpstate_active == 0
-*
-* # task uses the FPU and regs are active:
-* fpstate_active == 1 && fpregs_active == 1
-*
-* # the regs are inactive but still match fpstate:
-* fpstate_active == 1 && fpregs_active == 0 && fpregs_owner == fpu
-*
-* The third state is what we use for the lazy restore optimization
-* on lazy-switching CPUs.
-*/
- unsigned char fpregs_active;
-
- /*
* @state:
*
* In-memory copy of all FPU registers that we save/restore
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index 342e59789fcd..da565aae9fd2 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,7 +12,6 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_STRUCT__entry(
__field(struct fpu *, fpu)
- __field(bool, fpregs_active)
__field(bool, fpstate_active)
__field(u64, xfeatures)
__field(u64, xcomp_bv)
@@ -20,16 +19,14 @@ DECLARE_EVENT_CLASS(x86_fpu,
TP_fast_assign(
__entry->fpu= fpu;
- __entry->fpregs_active = fpu->fpregs_active;
__entry->fpstate_active = fpu->fpstate_active;
if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
__entry->xfeatures = fpu->state.xsave.header.xfeatures;
__entry->xcomp_bv = fpu->state.xsave.header.xcomp_bv;
}
),
- TP_printk("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures:
%llx xcomp_bv: %llx",
+ TP_printk("x86/fpu: %p fpstate_active: %d xfeatures: %llx xcomp_bv:
%llx",
__entry->fpu,
- __entry->fpregs_active,
__entry->fpstate_active,
__entry->xfeatures,
__entry->xcomp_bv
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 01a47e9edfb4..93103a909c47 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -147,8 +147,6 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
[PATCH 24/33] x86/fpu: Don't let userspace set bogus xcomp_bv
From: Eric Biggers
On x86, userspace can use the ptrace() or rt_sigreturn() system calls to
set a task's extended state (xstate) or "FPU" registers. ptrace() can
set them for another task using the PTRACE_SETREGSET request with
NT_X86_XSTATE, while rt_sigreturn() can set them for the current task.
In either case, registers can be set to any value, but the kernel
assumes that the XSAVE area itself remains valid in the sense that the
CPU can restore it.
However, in the case where the kernel is using the uncompacted xstate
format (which it does whenever the XSAVES instruction is unavailable),
it was possible for userspace to set the xcomp_bv field in the
xstate_header to an arbitrary value. However, all bits in that field
are reserved in the uncompacted case, so when switching to a task with
nonzero xcomp_bv, the XRSTOR instruction failed with a #GP fault. This
caused the WARN_ON_FPU(err) in copy_kernel_to_xregs() to be hit. In
addition, since the error is otherwise ignored, the FPU registers from
the task previously executing on the CPU were leaked.
Fix the bug by checking that the user-supplied value of xcomp_bv is 0 in
the uncompacted case, and returning an error otherwise.
The reason for validating xcomp_bv rather than simply overwriting it
with 0 is that we want userspace to see an error if it (incorrectly)
provides an XSAVE area in compacted format rather than in uncompacted
format.
Note that as before, in case of error we clear the task's FPU state.
This is perhaps non-ideal, especially for PTRACE_SETREGSET; it might be
better to return an error before changing anything. But it seems the
"clear on error" behavior is fine for now, and it's a little tricky to
do otherwise because it would mean we couldn't simply copy the full
userspace state into kernel memory in one __copy_from_user().
This bug was found by syzkaller, which hit the above-mentioned
WARN_ON_FPU():
WARNING: CPU: 1 PID: 0 at ./arch/x86/include/asm/fpu/internal.h:373
__switch_to+0x5b5/0x5d0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.13.0 #453
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 9ba2bc8e42c0 task.stack: a78cc036c000
RIP: 0010:__switch_to+0x5b5/0x5d0
RSP: :a78cc08bbb88 EFLAGS: 00010082
RAX: fffe RBX: 9ba2b8bf2180 RCX: c100
RDX: RSI: 5cb10700 RDI: 9ba2b8bf36c0
RBP: a78cc08bbbd0 R08: 929fdf46 R09: 0001
R10: R11: R12: 9ba2bc8e42c0
R13: R14: 9ba2b8bf3680 R15: 9ba2bf5d7b40
FS: 7f7e5cb10700() GS:9ba2bf40() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 004005cc CR3: 79fd5000 CR4: 001406e0
Call Trace:
Code: 84 00 00 00 00 00 e9 11 fd ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9
e7 fa ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 c2 fa ff ff <0f> ff 66 0f 1f 84
00 00 00 00 00 e9 d4 fc ff ff 66 66 2e 0f 1f
Here is a C reproducer. The expected behavior is that the program spin
forever with no output. However, on a buggy kernel running on a
processor with the "xsave" feature but without the "xsaves" feature
(e.g. Sandy Bridge through Broadwell for Intel), within a second or two
the program reports that the xmm registers were corrupted, i.e. were not
restored correctly. With CONFIG_X86_DEBUG_FPU=y it also hits the above
kernel warning.
#define _GNU_SOURCE
#include
#include
#include
#include
#include
#include
#include
#include
int main(void)
{
int pid = fork();
uint64_t xstate[512];
struct iovec iov = { .iov_base = xstate, .iov_len = sizeof(xstate) };
if (pid == 0) {
bool tracee = true;
for (int i = 0; i < sysconf(_SC_NPROCESSORS_ONLN) && tracee; i++)
tracee = (fork() != 0);
uint32_t xmm0[4] = { [0 ... 3] = tracee ? 0x : 0xDEADBEEF };
asm volatile(" movdqu %0, %%xmm0\n"
" mov %0, %%rbx\n"
"1: movdqu %%xmm0, %0\n"
" mov %0, %%rax\n"
" cmp %%rax, %%rbx\n"
" je 1b\n"
: "+m" (xmm0) : : "rax", "rbx", "xmm0");
printf("BUG: xmm registers corrupted! tracee=%d,
xmm0=%08X%08X%08X%08X\n",
tracee, xmm0[0], xmm0[1], xmm0[2], xmm0[3]);
} else {
usleep(10);
ptrace(PTRACE_ATTACH, pid, 0, 0);
wait(NULL);
ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov);
xstate[65] = -1;
ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov);
ptrace(PTRACE_CONT, pid, 0, 0);
wait(NULL);
}
return 1;
}
Note: the program only tests for the bug using the ptrace() system c
[PATCH 22/33] x86/fpu: Fix boolreturn.cocci warnings
From: kbuild test robot
arch/x86/kernel/fpu/xstate.c:931:9-10: WARNING: return of 0/1 in function
'xfeatures_mxcsr_quirk' with return type bool
Return statements in functions returning bool should use true/false instead of
1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci
Signed-off-by: Fengguang Wu
Signed-off-by: Thomas Gleixner
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Yu-cheng Yu
Cc: kbuild-...@01.org
Cc: tipbu...@zytor.com
Link: http://lkml.kernel.org/r/20170306004553.GA25764@lkp-wsm-ep1
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/xstate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 41c52256bdce..fda1109cc355 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -929,12 +929,12 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
int pkey,
static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
{
if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
- return 0;
+ return false;
if (xfeatures & XFEATURE_MASK_FP)
- return 0;
+ return false;
- return 1;
+ return true;
}
/*
--
2.11.0
[PATCH 15/33] x86/fpu: Simplify fpu->fpregs_active use
The fpregs_active() inline function is pretty pointless - in almost
all the callsites it can be replaced with a direct fpu->fpregs_active
access.
Do so and eliminate the extra layer of obfuscation.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 17 +
arch/x86/kernel/fpu/core.c | 2 +-
arch/x86/kernel/fpu/signal.c| 9 +
arch/x86/mm/pkeys.c | 2 +-
4 files changed, 8 insertions(+), 22 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 554cdb205d17..b223c57dd5dc 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -543,21 +543,6 @@ static inline void fpregs_activate(struct fpu *fpu)
}
/*
- * The question "does this thread have fpu access?"
- * is slightly racy, since preemption could come in
- * and revoke it immediately after the test.
- *
- * However, even in that very unlikely scenario,
- * we can just assume we have FPU access - typically
- * to save the FP state - we'll just take a #NM
- * fault and get the FPU access back.
- */
-static inline int fpregs_active(void)
-{
- return current->thread.fpu.fpregs_active;
-}
-
-/*
* FPU state switching for scheduling.
*
* This is a two-stage process:
@@ -617,7 +602,7 @@ static inline void user_fpu_begin(void)
struct fpu *fpu = ¤t->thread.fpu;
preempt_disable();
- if (!fpregs_active())
+ if (!fpu->fpregs_active)
fpregs_activate(fpu);
preempt_enable();
}
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index e1114f070c2d..bad57248e5a0 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -367,7 +367,7 @@ void fpu__current_fpstate_write_end(void)
* registers may still be out of date. Update them with
* an XRSTOR if they are active.
*/
- if (fpregs_active())
+ if (fpu->fpregs_active)
copy_kernel_to_fpregs(&fpu->state);
/*
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 2d682dac35d4..684025654d0c 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -155,7 +155,8 @@ static inline int copy_fpregs_to_sigframe(struct
xregs_state __user *buf)
*/
int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
{
- struct xregs_state *xsave = ¤t->thread.fpu.state.xsave;
+ struct fpu *fpu = ¤t->thread.fpu;
+ struct xregs_state *xsave = &fpu->state.xsave;
struct task_struct *tsk = current;
int ia32_fxstate = (buf != buf_fx);
@@ -170,13 +171,13 @@ int copy_fpstate_to_sigframe(void __user *buf, void
__user *buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- if (fpregs_active() || using_compacted_format()) {
+ if (fpu->fpregs_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
return -1;
/* Update the thread's fxstate to save the fsave header. */
if (ia32_fxstate)
- copy_fxregs_to_kernel(&tsk->thread.fpu);
+ copy_fxregs_to_kernel(fpu);
} else {
/*
* It is a *bug* if kernel uses compacted-format for xsave
@@ -189,7 +190,7 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user
*buf_fx, int size)
return -1;
}
- fpstate_sanitize_xstate(&tsk->thread.fpu);
+ fpstate_sanitize_xstate(fpu);
if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size))
return -1;
}
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index 2dab69a706ec..e2c23472233e 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -45,7 +45,7 @@ int __execute_only_pkey(struct mm_struct *mm)
*/
preempt_disable();
if (!need_to_set_mm_pkey &&
- fpregs_active() &&
+ current->thread.fpu.fpregs_active &&
!__pkru_allows_read(read_pkru(), execute_only_pkey)) {
preempt_enable();
return execute_only_pkey;
--
2.11.0
[PATCH 09/33] x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*()
'size_total' is derived from an unsigned input parameter - and then converted
to 'int' and checked for negative ranges:
if (size_total < 0 || offset < size_total) {
This conversion and the checks are unnecessary obfuscation, reject overly
large requested copy sizes outright and simplify the underlying code.
Reported-by: Rik van Riel
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/xstate.c | 32 +++-
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c13083579655..b18c5457065a 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -925,15 +925,11 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
int pkey,
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
static inline int
-__copy_xstate_to_kernel(void *kbuf,
- const void *data,
- unsigned int offset, unsigned int size, int size_total)
+__copy_xstate_to_kernel(void *kbuf, const void *data,
+ unsigned int offset, unsigned int size, unsigned int
size_total)
{
- if (!size)
- return 0;
-
- if (size_total < 0 || offset < size_total) {
- unsigned int copy = size_total < 0 ? size : min(size,
size_total - offset);
+ if (offset < size_total) {
+ unsigned int copy = min(size, size_total - offset);
memcpy(kbuf + offset, data, copy);
}
@@ -986,12 +982,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
offset = xstate_offsets[i];
size = xstate_sizes[i];
+ /* The next component has to fit fully into the output
buffer: */
+ if (offset + size > size_total)
+ break;
+
ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
size_total);
if (ret)
return ret;
-
- if (offset + size >= size_total)
- break;
}
}
@@ -1010,13 +1007,13 @@ int copy_xstate_to_kernel(void *kbuf, struct
xregs_state *xsave, unsigned int of
}
static inline int
-__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int
offset, unsigned int size, int size_total)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int
offset, unsigned int size, unsigned int size_total)
{
if (!size)
return 0;
- if (size_total < 0 || offset < size_total) {
- unsigned int copy = size_total < 0 ? size : min(size,
size_total - offset);
+ if (offset < size_total) {
+ unsigned int copy = min(size, size_total - offset);
if (__copy_to_user(ubuf + offset, data, copy))
return -EFAULT;
@@ -1069,12 +1066,13 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
offset = xstate_offsets[i];
size = xstate_sizes[i];
+ /* The next component has to fit fully into the output
buffer: */
+ if (offset + size > size_total)
+ break;
+
ret = __copy_xstate_to_user(ubuf, src, offset, size,
size_total);
if (ret)
return ret;
-
- if (offset + size >= size_total)
- break;
}
}
--
2.11.0
[PATCH 14/33] x86/fpu: Flip the parameter order in copy_*_to_xstate()
Make it more consistent with regular memcpy() semantics, where the destination
argument comes first.
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 4 ++--
arch/x86/kernel/fpu/signal.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 4 ++--
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 4ceb90740d80..579ac2358e63 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -50,6 +50,6 @@ const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset, unsigned int size);
int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
-int copy_kernel_to_xstate(const void *kbuf, struct xregs_state *xsave);
-int copy_user_to_xstate(const void __user *ubuf, struct xregs_state *xsave);
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index caf723f31737..19a7385a912c 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -136,9 +136,9 @@ int xstateregs_set(struct task_struct *target, const struct
user_regset *regset,
if (boot_cpu_has(X86_FEATURE_XSAVES)) {
if (kbuf)
- ret = copy_kernel_to_xstate(kbuf, xsave);
+ ret = copy_kernel_to_xstate(xsave, kbuf);
else
- ret = copy_user_to_xstate(ubuf, xsave);
+ ret = copy_user_to_xstate(xsave, ubuf);
} else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0,
-1);
}
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 2c685b492fd6..2d682dac35d4 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -324,7 +324,7 @@ static int __fpu__restore_sig(void __user *buf, void __user
*buf_fx, int size)
fpu__drop(fpu);
if (using_compacted_format())
- err = copy_user_to_xstate(buf_fx, &fpu->state.xsave);
+ err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
else
err = __copy_from_user(&fpu->state.xsave, buf_fx,
state_size);
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b1f3e4dae2e3..0ef35040d0ad 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1089,7 +1089,7 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
*/
-int copy_kernel_to_xstate(const void *kbuf, struct xregs_state *xsave)
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
{
unsigned int offset, size;
int i;
@@ -1142,7 +1142,7 @@ int copy_kernel_to_xstate(const void *kbuf, struct
xregs_state *xsave)
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
*/
-int copy_user_to_xstate(const void __user *ubuf, struct xregs_state *xsave)
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
{
unsigned int offset, size;
int i;
--
2.11.0
[PATCH 12/33] x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 17 +++--
3 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 79af79dbcab6..f10889bc0c88 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -50,6 +50,6 @@ const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset, unsigned int size);
int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
-int copy_kernel_to_xstate(const void *kbuf, const void __user *ubuf, struct
xregs_state *xsave);
+int copy_kernel_to_xstate(const void *kbuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf, struct
xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index cb45dd81d617..785302c75f38 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -136,7 +136,7 @@ int xstateregs_set(struct task_struct *target, const struct
user_regset *regset,
if (boot_cpu_has(X86_FEATURE_XSAVES)) {
if (kbuf)
- ret = copy_kernel_to_xstate(kbuf, ubuf, xsave);
+ ret = copy_kernel_to_xstate(kbuf, xsave);
else
ret = copy_user_to_xstate(kbuf, ubuf, xsave);
} else {
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 1ad25d1b8056..71cc8d367fdd 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1089,8 +1089,7 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
*/
-int copy_kernel_to_xstate(const void *kbuf, const void __user *ubuf,
-struct xregs_state *xsave)
+int copy_kernel_to_xstate(const void *kbuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int i;
@@ -1100,12 +1099,7 @@ int copy_kernel_to_xstate(const void *kbuf, const void
__user *ubuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(xfeatures);
- if (kbuf) {
- memcpy(&xfeatures, kbuf + offset, size);
- } else {
- if (__copy_from_user(&xfeatures, ubuf + offset, size))
- return -EFAULT;
- }
+ memcpy(&xfeatures, kbuf + offset, size);
/*
* Reject if the user sets any disabled or supervisor features:
@@ -1124,12 +1118,7 @@ int copy_kernel_to_xstate(const void *kbuf, const void
__user *ubuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- if (kbuf) {
- memcpy(dst, kbuf + offset, size);
- } else {
- if (__copy_from_user(dst, ubuf + offset, size))
- return -EFAULT;
- }
+ memcpy(dst, kbuf + offset, size);
}
}
--
2.11.0
[PATCH 16/33] x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic
Do this temporarily only, to make it easier to change the FPU state machine,
in particular this change couples the fpu->fpregs_active and fpu->fpstate_active
states: they are only set/cleared together (as far as the scheduler sees them).
This will be removed by later patches.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index bad57248e5a0..b7dc3833d41a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -462,9 +462,11 @@ void fpu__clear(struct fpu *fpu)
* Make sure fpstate is cleared and initialized.
*/
if (static_cpu_has(X86_FEATURE_FPU)) {
+ preempt_disable();
fpu__activate_curr(fpu);
user_fpu_begin();
copy_init_fpstate_to_fpregs();
+ preempt_enable();
}
}
--
2.11.0
[PATCH 10/33] x86/fpu: Simplify __copy_xstate_to_kernel() return values
__copy_xstate_to_kernel() can only return 0 (because kernel copies cannot fail),
simplify the code throughout.
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/xstate.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b18c5457065a..00c3b41c3cf1 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -924,7 +924,7 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
* This is similar to user_regset_copyout(), but will not add offset to
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
-static inline int
+static inline void
__copy_xstate_to_kernel(void *kbuf, const void *data,
unsigned int offset, unsigned int size, unsigned int
size_total)
{
@@ -933,7 +933,6 @@ __copy_xstate_to_kernel(void *kbuf, const void *data,
memcpy(kbuf + offset, data, copy);
}
- return 0;
}
/*
@@ -946,8 +945,8 @@ __copy_xstate_to_kernel(void *kbuf, const void *data,
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset_start, unsigned int size_total)
{
unsigned int offset, size;
- int ret, i;
struct xstate_header header;
+ int i;
/*
* Currently copy_regset_to_user() starts from pos 0:
@@ -968,9 +967,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
- if (ret)
- return ret;
+ __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
for (i = 0; i < XFEATURE_MAX; i++) {
/*
@@ -986,9 +983,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
if (offset + size > size_total)
break;
- ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
size_total);
- if (ret)
- return ret;
+ __copy_xstate_to_kernel(kbuf, src, offset, size,
size_total);
}
}
@@ -999,9 +994,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size,
size_total);
- if (ret)
- return ret;
+ __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size,
size_total);
return 0;
}
--
2.11.0
Re: [PATCH v2 0/2] x86: Fix inline asm call constraints for clang
* Josh Poimboeuf wrote: > On Sat, Sep 23, 2017 at 12:53:18PM +0200, Ingo Molnar wrote: > > > > * Josh Poimboeuf wrote: > > > > > Patch 1 is a bug fix for an objtool issue which was uncovered by patch 2. > > > > > > Patch 2 is the last fix needed for clang to be able to compile and boot > > > the kernel. > > > > > > Josh Poimboeuf (2): > > > objtool: Handle another GCC stack pointer adjustment bug > > > x86/asm: Fix inline asm call constraints for clang > > > > Have these patches been boot tested with clang? > > Yes, I built and boot them using the directions at: > > https://github.com/ramosian-glider/clang-kernel-build Great - I'll queue up these fixes in tip:x86/urgent. Thanks, Ingo
[PATCH 11/33] x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate()
Similar to:
x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() &
copy_xstate_to_user()
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 10 ++---
arch/x86/kernel/fpu/xstate.c | 66
++-
3 files changed, 74 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index fed6617a1079..79af79dbcab6 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -50,6 +50,6 @@ const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset, unsigned int size);
int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
-int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
-struct xregs_state *xsave);
+int copy_kernel_to_xstate(const void *kbuf, const void __user *ubuf, struct
xregs_state *xsave);
+int copy_user_to_xstate(const void *kbuf, const void __user *ubuf, struct
xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index ec1404194b65..cb45dd81d617 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -134,10 +134,14 @@ int xstateregs_set(struct task_struct *target, const
struct user_regset *regset,
fpu__activate_fpstate_write(fpu);
- if (boot_cpu_has(X86_FEATURE_XSAVES))
- ret = copy_user_to_xstate(kbuf, ubuf, xsave);
- else
+ if (boot_cpu_has(X86_FEATURE_XSAVES)) {
+ if (kbuf)
+ ret = copy_kernel_to_xstate(kbuf, ubuf, xsave);
+ else
+ ret = copy_user_to_xstate(kbuf, ubuf, xsave);
+ } else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0,
-1);
+ }
/*
* In case of failure, mark all states as init:
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 00c3b41c3cf1..1ad25d1b8056 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1084,7 +1084,71 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
}
/*
- * Convert from a ptrace standard-format buffer to kernel XSAVES format
+ * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format
+ * and copy to the target thread. This is called from xstateregs_set() and
+ * there we check the CPU has XSAVES and a whole standard-sized buffer
+ * exists.
+ */
+int copy_kernel_to_xstate(const void *kbuf, const void __user *ubuf,
+struct xregs_state *xsave)
+{
+ unsigned int offset, size;
+ int i;
+ u64 xfeatures;
+ u64 allowed_features;
+
+ offset = offsetof(struct xregs_state, header);
+ size = sizeof(xfeatures);
+
+ if (kbuf) {
+ memcpy(&xfeatures, kbuf + offset, size);
+ } else {
+ if (__copy_from_user(&xfeatures, ubuf + offset, size))
+ return -EFAULT;
+ }
+
+ /*
+* Reject if the user sets any disabled or supervisor features:
+*/
+ allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
+
+ if (xfeatures & ~allowed_features)
+ return -EINVAL;
+
+ for (i = 0; i < XFEATURE_MAX; i++) {
+ u64 mask = ((u64)1 << i);
+
+ if (xfeatures & mask) {
+ void *dst = __raw_xsave_addr(xsave, 1 << i);
+
+ offset = xstate_offsets[i];
+ size = xstate_sizes[i];
+
+ if (kbuf) {
+ memcpy(dst, kbuf + offset, size);
+ } else {
+ if (__copy_from_user(dst, ubuf + offset, size))
+ return -EFAULT;
+ }
+ }
+ }
+
+ /*
+* The state that came in from userspace was user-state only.
+* Mask all the user states out of 'xfeatures':
+*/
+ xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
+
+ /*
+* Add back in the features that came in from userspace:
+*/
+ xsave->header.xfeatures |= xfeatures;
+
+ return 0;
+}
+
+/*
+ * Convert from a ptrace standard-format user-space buffer to kernel XSAVES
format
* and copy to the target thread. This is called from xstateregs_set() and
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
--
2.11
[PATCH 19/33] x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active
The fpregs_activate()/fpregs_deactivate() are currently called in such a
pattern:
if (!fpu->fpregs_active)
fpregs_activate(fpu);
...
if (fpu->fpregs_active)
fpregs_deactivate(fpu);
But note that it's actually safe to call them without checking the flag first.
This further decouples the fpu->fpregs_active flag from actual FPU logic.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 7 +--
arch/x86/kernel/fpu/core.c | 3 +--
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index 7fa676f93ac1..42a601673c09 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -526,8 +526,6 @@ static inline int fpregs_state_valid(struct fpu *fpu,
unsigned int cpu)
*/
static inline void fpregs_deactivate(struct fpu *fpu)
{
- WARN_ON_FPU(!fpu->fpregs_active);
-
fpu->fpregs_active = 0;
this_cpu_write(fpu_fpregs_owner_ctx, NULL);
trace_x86_fpu_regs_deactivated(fpu);
@@ -535,8 +533,6 @@ static inline void fpregs_deactivate(struct fpu *fpu)
static inline void fpregs_activate(struct fpu *fpu)
{
- WARN_ON_FPU(fpu->fpregs_active);
-
fpu->fpregs_active = 1;
this_cpu_write(fpu_fpregs_owner_ctx, fpu);
trace_x86_fpu_regs_activated(fpu);
@@ -604,8 +600,7 @@ static inline void user_fpu_begin(void)
struct fpu *fpu = ¤t->thread.fpu;
preempt_disable();
- if (!fpu->fpregs_active)
- fpregs_activate(fpu);
+ fpregs_activate(fpu);
preempt_enable();
}
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index eab244622402..01a47e9edfb4 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -426,8 +426,7 @@ void fpu__drop(struct fpu *fpu)
asm volatile("1: fwait\n"
"2:\n"
_ASM_EXTABLE(1b, 2b));
- if (fpu->fpregs_active)
- fpregs_deactivate(fpu);
+ fpregs_deactivate(fpu);
}
} else {
WARN_ON_FPU(fpu->fpregs_active);
--
2.11.0
[PATCH 13/33] x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/signal.c | 11 ---
arch/x86/kernel/fpu/xstate.c | 19 +--
4 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index f10889bc0c88..4ceb90740d80 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -51,5 +51,5 @@ int using_compacted_format(void);
int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset, unsigned int size);
int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
int copy_kernel_to_xstate(const void *kbuf, struct xregs_state *xsave);
-int copy_user_to_xstate(const void *kbuf, const void __user *ubuf, struct
xregs_state *xsave);
+int copy_user_to_xstate(const void __user *ubuf, struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 785302c75f38..caf723f31737 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -138,7 +138,7 @@ int xstateregs_set(struct task_struct *target, const struct
user_regset *regset,
if (kbuf)
ret = copy_kernel_to_xstate(kbuf, xsave);
else
- ret = copy_user_to_xstate(kbuf, ubuf, xsave);
+ ret = copy_user_to_xstate(ubuf, xsave);
} else {
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0,
-1);
}
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index b1fe9a1fc4e0..2c685b492fd6 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -323,13 +323,10 @@ static int __fpu__restore_sig(void __user *buf, void
__user *buf_fx, int size)
*/
fpu__drop(fpu);
- if (using_compacted_format()) {
- err = copy_user_to_xstate(NULL, buf_fx,
- &fpu->state.xsave);
- } else {
- err = __copy_from_user(&fpu->state.xsave,
- buf_fx, state_size);
- }
+ if (using_compacted_format())
+ err = copy_user_to_xstate(buf_fx, &fpu->state.xsave);
+ else
+ err = __copy_from_user(&fpu->state.xsave, buf_fx,
state_size);
if (err || __copy_from_user(&env, buf, sizeof(env))) {
fpstate_init(&fpu->state);
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 71cc8d367fdd..b1f3e4dae2e3 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1142,8 +1142,7 @@ int copy_kernel_to_xstate(const void *kbuf, struct
xregs_state *xsave)
* there we check the CPU has XSAVES and a whole standard-sized buffer
* exists.
*/
-int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
-struct xregs_state *xsave)
+int copy_user_to_xstate(const void __user *ubuf, struct xregs_state *xsave)
{
unsigned int offset, size;
int i;
@@ -1153,12 +1152,8 @@ int copy_user_to_xstate(const void *kbuf, const void
__user *ubuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(xfeatures);
- if (kbuf) {
- memcpy(&xfeatures, kbuf + offset, size);
- } else {
- if (__copy_from_user(&xfeatures, ubuf + offset, size))
- return -EFAULT;
- }
+ if (__copy_from_user(&xfeatures, ubuf + offset, size))
+ return -EFAULT;
/*
* Reject if the user sets any disabled or supervisor features:
@@ -1177,12 +1172,8 @@ int copy_user_to_xstate(const void *kbuf, const void
__user *ubuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- if (kbuf) {
- memcpy(dst, kbuf + offset, size);
- } else {
- if (__copy_from_user(dst, ubuf + offset, size))
- return -EFAULT;
- }
+ if (__copy_from_user(dst, ubuf + offset, size))
+ return -EFAULT;
}
}
--
2.11.0
[PATCH 17/33] x86/fpu: Split the state handling in fpu__drop()
Prepare fpu__drop() to use fpu->fpregs_active.
There are two distinct usecases for fpu__drop() in this context:
exit_thread() when called for 'current' in exit(), and when called
for another task in fork().
This patch does not change behavior, it only adds a couple of
debug checks and structures the code to make the ->fpregs_active
change more obviously correct.
All the complications will be removed later on.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/core.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index b7dc3833d41a..815dfba7781a 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -414,12 +414,19 @@ void fpu__drop(struct fpu *fpu)
{
preempt_disable();
- if (fpu->fpregs_active) {
- /* Ignore delayed exceptions from user space */
- asm volatile("1: fwait\n"
-"2:\n"
-_ASM_EXTABLE(1b, 2b));
- fpregs_deactivate(fpu);
+ if (fpu == ¤t->thread.fpu) {
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
+ if (fpu->fpregs_active) {
+ /* Ignore delayed exceptions from user space */
+ asm volatile("1: fwait\n"
+"2:\n"
+_ASM_EXTABLE(1b, 2b));
+ if (fpu->fpregs_active)
+ fpregs_deactivate(fpu);
+ }
+ } else {
+ WARN_ON_FPU(fpu->fpregs_active);
}
fpu->fpstate_active = 0;
--
2.11.0
[PATCH 18/33] x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
We want to simplify the FPU state machine by eliminating fpu->fpregs_active,
and we can do that because the two state flags (::fpregs_active and
::fpstate_active) are set essentially together.
The old lazy FPU switching code used to make a distinction - but there's
no lazy switching code anymore, we always switch in an 'eager' fashion.
Do this by first changing all substantial uses of fpu->fpregs_active
to fpu->fpstate_active and adding a few debug checks to double check
our assumption is correct.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/internal.h | 4 +++-
arch/x86/kernel/fpu/core.c | 16 ++--
arch/x86/kernel/fpu/signal.c| 4 +++-
arch/x86/mm/pkeys.c | 3 +--
4 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/fpu/internal.h
b/arch/x86/include/asm/fpu/internal.h
index b223c57dd5dc..7fa676f93ac1 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -556,7 +556,9 @@ static inline void fpregs_activate(struct fpu *fpu)
static inline void
switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
- if (old_fpu->fpregs_active) {
+ WARN_ON_FPU(old_fpu->fpregs_active != old_fpu->fpstate_active);
+
+ if (old_fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(old_fpu))
old_fpu->last_cpu = -1;
else
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 815dfba7781a..eab244622402 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -100,7 +100,7 @@ void __kernel_fpu_begin(void)
kernel_fpu_disable();
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/*
* Ignore return value -- we don't care if reg state
* is clobbered.
@@ -116,7 +116,7 @@ void __kernel_fpu_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
kernel_fpu_enable();
@@ -147,8 +147,10 @@ void fpu__save(struct fpu *fpu)
WARN_ON_FPU(fpu != ¤t->thread.fpu);
preempt_disable();
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
trace_x86_fpu_before_save(fpu);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
if (!copy_fpregs_to_fpstate(fpu)) {
copy_kernel_to_fpregs(&fpu->state);
}
@@ -262,11 +264,12 @@ EXPORT_SYMBOL_GPL(fpu__activate_curr);
*/
void fpu__activate_fpstate_read(struct fpu *fpu)
{
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* If fpregs are active (in the current CPU), then
* copy them to the fpstate:
*/
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
fpu__save(fpu);
} else {
if (!fpu->fpstate_active) {
@@ -362,12 +365,13 @@ void fpu__current_fpstate_write_end(void)
{
struct fpu *fpu = ¤t->thread.fpu;
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
/*
* 'fpu' now has an updated copy of the state, but the
* registers may still be out of date. Update them with
* an XRSTOR if they are active.
*/
- if (fpu->fpregs_active)
+ if (fpu->fpstate_active)
copy_kernel_to_fpregs(&fpu->state);
/*
@@ -417,7 +421,7 @@ void fpu__drop(struct fpu *fpu)
if (fpu == ¤t->thread.fpu) {
WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
- if (fpu->fpregs_active) {
+ if (fpu->fpstate_active) {
/* Ignore delayed exceptions from user space */
asm volatile("1: fwait\n"
"2:\n"
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 684025654d0c..a88083ba7f8b 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -171,7 +171,9 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user
*buf_fx, int size)
sizeof(struct user_i387_ia32_struct), NULL,
(struct _fpstate_32 __user *) buf) ? -1 : 1;
- if (fpu->fpregs_active || using_compacted_format()) {
+ WARN_ON_FPU(fpu->fpstate_active != fpu->fpregs_active);
+
+ if (fpu->fpstate_active || using_compacted_format()) {
/* Save the live register state to the user directly. */
if (copy_fpregs_to_sigframe(buf_fx))
return -1;
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index e2c23472233e..4d24269c071f 100644
--
[PATCH 08/33] x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods
Right now there's a confusing mixture of 'offset' and 'size' parameters:
- __copy_xstate_to_*() input parameter 'end_pos' not not really an offset,
but the full size of the copy to be performed.
- input parameter 'count' to copy_xstate_to_*() shadows that of
__copy_xstate_to_*()'s 'count' parameter name - but the roles
are different: the first one is the total number of bytes to
be copied, while the second one is a partial copy size.
To unconfuse all this, use a consistent set of parameter names:
- 'size' is the partial copy size within a single xstate component
- 'size_total' is the total copy requested
- 'offset_start' is the requested starting offset.
- 'offset' is the offset within an xstate component.
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/xstate.c | 44
++--
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index e4430b84939d..fed6617a1079 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
pos, unsigned int count);
-int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int pos, unsigned int count);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset, unsigned int size);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int offset, unsigned int size);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 1f50fdaf4c5a..c13083579655 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -927,15 +927,15 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
int pkey,
static inline int
__copy_xstate_to_kernel(void *kbuf,
const void *data,
- unsigned int pos, unsigned int count, int end_pos)
+ unsigned int offset, unsigned int size, int size_total)
{
- if (!count)
+ if (!size)
return 0;
- if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = end_pos < 0 ? count : min(count, end_pos -
pos);
+ if (size_total < 0 || offset < size_total) {
+ unsigned int copy = size_total < 0 ? size : min(size,
size_total - offset);
- memcpy(kbuf + pos, data, copy);
+ memcpy(kbuf + offset, data, copy);
}
return 0;
}
@@ -947,7 +947,7 @@ __copy_xstate_to_kernel(void *kbuf,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
pos, unsigned int count)
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
offset_start, unsigned int size_total)
{
unsigned int offset, size;
int ret, i;
@@ -956,7 +956,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
/*
* Currently copy_regset_to_user() starts from pos 0:
*/
- if (unlikely(pos != 0))
+ if (unlikely(offset_start != 0))
return -EFAULT;
/*
@@ -972,7 +972,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, count);
+ ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
if (ret)
return ret;
@@ -986,11 +986,11 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
count);
+ ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
size_total);
if (ret)
return ret;
- if (offset + size >= count)
+ if (offset + size >= size_total)
break;
}
@@ -1002,7
[PATCH 07/33] x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions
'start_pos' is always 0, so remove it and remove the pointless check of 'pos <
0'
which can not ever be true as 'pos' is unsigned ...
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/xstate.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 9647e7256179..1f50fdaf4c5a 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -927,9 +927,9 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
static inline int
__copy_xstate_to_kernel(void *kbuf,
const void *data,
- unsigned int pos, unsigned int count, int start_pos,
int end_pos)
+ unsigned int pos, unsigned int count, int end_pos)
{
- if ((count == 0) || (pos < start_pos))
+ if (!count)
return 0;
if (end_pos < 0 || pos < end_pos) {
@@ -972,7 +972,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, count);
if (ret)
return ret;
@@ -986,7 +986,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
0, count);
+ ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
count);
if (ret)
return ret;
@@ -1002,7 +1002,7 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size,
0, count);
+ ret = __copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size,
count);
if (ret)
return ret;
@@ -1010,9 +1010,9 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int po
}
static inline int
-__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos,
unsigned int count, int start_pos, int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos,
unsigned int count, int end_pos)
{
- if ((count == 0) || (pos < start_pos))
+ if (!count)
return 0;
if (end_pos < 0 || pos < end_pos) {
@@ -1055,7 +1055,7 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_user(ubuf, &header, offset, size, 0, count);
+ ret = __copy_xstate_to_user(ubuf, &header, offset, size, count);
if (ret)
return ret;
@@ -1069,7 +1069,7 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_user(ubuf, src, offset, size, 0,
count);
+ ret = __copy_xstate_to_user(ubuf, src, offset, size,
count);
if (ret)
return ret;
@@ -1085,7 +1085,7 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, 0,
count);
+ ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size,
count);
if (ret)
return ret;
--
2.11.0
[PATCH 05/33] x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
Parameter ordering is weird:
int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user
*ubuf, struct xregs_state *xsave);
'pos' and 'count', which are attributes of the destination buffer, are listed
before the destination
buffer itself ...
List them after the primary arguments instead.
This makes the code more similar to regular memcpy() variant APIs.
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 ++--
arch/x86/kernel/fpu/regset.c | 4 ++--
arch/x86/kernel/fpu/xstate.c | 25 -
3 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 65bd68c30cd0..e4430b84939d 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void __user
*ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
pos, unsigned int count);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned
int pos, unsigned int count);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index fd6dbdd8fde6..ec1404194b65 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,9 +93,9 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
+ ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
else
- ret = copy_xstate_to_user(pos, count, ubuf, xsave);
+ ret = copy_xstate_to_user(ubuf, xsave, pos, count);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2d8f3344875d..0a299468510f 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -925,10 +925,9 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
static inline int
-__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf,
- const void *data, const int start_pos,
- const int end_pos)
+__copy_xstate_to_kernel(void *kbuf,
+ const void *data,
+ unsigned int pos, unsigned int count, const int
start_pos, const int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
@@ -948,7 +947,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int
count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave)
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int
pos, unsigned int count)
{
unsigned int offset, size;
int ret, i;
@@ -973,7 +972,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf, stru
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
+ ret = __copy_xstate_to_kernel(kbuf, &header, offset, size, 0, count);
if (ret)
return ret;
@@ -987,7 +986,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf, stru
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, src,
0, count);
+ ret = __copy_xstate_to_kernel(kbuf, src, offset, size,
0, count);
if (ret)
return ret;
@@ -1003,7 +1002,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf, stru
off
[PATCH 06/33] x86/fpu: Clean up the parameter definitions of copy_xstate_to_*()
Remove pointless 'const' of non-pointer input parameter.
Remove unnecessary parenthesis that shows uncertainty about arithmetic operator
precedence.
Clarify copy_xstate_to_user() description.
No change in functionality.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/kernel/fpu/xstate.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0a299468510f..9647e7256179 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -927,13 +927,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
int pkey,
static inline int
__copy_xstate_to_kernel(void *kbuf,
const void *data,
- unsigned int pos, unsigned int count, const int
start_pos, const int end_pos)
+ unsigned int pos, unsigned int count, int start_pos,
int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos -
pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos -
pos);
memcpy(kbuf + pos, data, copy);
}
@@ -1010,13 +1010,13 @@ int copy_xstate_to_kernel(void *kbuf, struct
xregs_state *xsave, unsigned int po
}
static inline int
-__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos,
unsigned int count, const int start_pos, const int end_pos)
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int pos,
unsigned int count, int start_pos, int end_pos)
{
if ((count == 0) || (pos < start_pos))
return 0;
if (end_pos < 0 || pos < end_pos) {
- unsigned int copy = (end_pos < 0 ? count : min(count, end_pos -
pos));
+ unsigned int copy = end_pos < 0 ? count : min(count, end_pos -
pos);
if (__copy_to_user(ubuf + pos, data, copy))
return -EFAULT;
@@ -1026,7 +1026,7 @@ __copy_xstate_to_user(void __user *ubuf, const void
*data, unsigned int pos, uns
/*
* Convert from kernel XSAVES compacted format to standard format and copy
- * to a ptrace buffer. It supports partial copy but pos always starts from
+ * to a user-space buffer. It supports partial copy but pos always starts from
* zero. This is called from xstateregs_get() and there we check the CPU
* has XSAVES.
*/
--
2.11.0
[PATCH 03/33] x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs
The 'ubuf' parameter is unused in the _kernel() side of the API, remove it.
This simplifies the code and makes it easier to think about.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 2 +-
arch/x86/kernel/fpu/regset.c | 2 +-
arch/x86/kernel/fpu/xstate.c | 21 ++---
3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index 92dc8ca14124..c762574a245f 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,7 +48,7 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave);
int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void
__user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b6d12d66d04b..34e74adf9d5d 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -93,7 +93,7 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
if (using_compacted_format()) {
if (kbuf)
- ret = copy_xstate_to_kernel(pos, count, kbuf, ubuf,
xsave);
+ ret = copy_xstate_to_kernel(pos, count, kbuf, xsave);
else
ret = copy_xstate_to_user(pos, count, kbuf, ubuf,
xsave);
} else {
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 38561539cb99..71d3bda2b898 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -926,7 +926,7 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
*/
static inline int
__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
- void *kbuf, void __user *ubuf,
+ void *kbuf,
const void *data, const int start_pos,
const int end_pos)
{
@@ -936,12 +936,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int
count,
if (end_pos < 0 || pos < end_pos) {
unsigned int copy = (end_pos < 0 ? count : min(count, end_pos -
pos));
- if (kbuf) {
- memcpy(kbuf + pos, data, copy);
- } else {
- if (__copy_to_user(ubuf + pos, data, copy))
- return -EFAULT;
- }
+ memcpy(kbuf + pos, data, copy);
}
return 0;
}
@@ -953,8 +948,7 @@ __copy_xstate_to_kernel(unsigned int pos, unsigned int
count,
* It supports partial copy but pos always starts from zero. This is called
* from xstateregs_get() and there we check the CPU has XSAVES.
*/
-int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
struct xregs_state *xsave)
{
unsigned int offset, size;
int ret, i;
@@ -979,8 +973,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf,
offset = offsetof(struct xregs_state, header);
size = sizeof(header);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf, &header, 0,
count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, &header, 0, count);
if (ret)
return ret;
@@ -994,8 +987,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf,
offset = xstate_offsets[i];
size = xstate_sizes[i];
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf,
src, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, src,
0, count);
if (ret)
return ret;
@@ -1011,8 +1003,7 @@ int copy_xstate_to_kernel(unsigned int pos, unsigned int
count, void *kbuf,
offset = offsetof(struct fxregs_state, sw_reserved);
size = sizeof(xstate_fx_sw_bytes);
- ret = __copy_xstate_to_kernel(offset, size, kbuf, ubuf,
xstate_fx_sw_bytes, 0, count);
-
+ ret = __copy_xstate_to_kernel(offset, size, kbuf, xstate_fx_sw_bytes,
0, count);
[PATCH 02/33] x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user()
copy_xstate_to_user() is a weird API - in part due to a bad API inherited
from the regset APIs.
But don't propagate that bad API choice into the FPU code - so as a first
step split the API into kernel and user buffer handling routines.
(Also split the xstate_copyout() internal helper.)
The split API is a dumb duplication that should be obviously correct, the
real splitting will be done in the next patch.
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Rik van Riel
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/xstate.h | 4 +--
arch/x86/kernel/fpu/regset.c | 5 ++-
arch/x86/kernel/fpu/xstate.c | 110
++
3 files changed, 109 insertions(+), 10 deletions(-)
diff --git a/arch/x86/include/asm/fpu/xstate.h
b/arch/x86/include/asm/fpu/xstate.h
index a1baa17e9748..92dc8ca14124 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,8 @@ void fpu__xstate_clear_all_cpu_caps(void);
void *get_xsave_addr(struct xregs_state *xsave, int xstate);
const void *get_xsave_field_ptr(int xstate_field);
int using_compacted_format(void);
-int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf,
- void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
void __user *ubuf, struct xregs_state *xsave);
+int copy_xstate_to_user(unsigned int pos, unsigned int count, void *kbuf, void
__user *ubuf, struct xregs_state *xsave);
int copy_user_to_xstate(const void *kbuf, const void __user *ubuf,
struct xregs_state *xsave);
#endif
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 165d0545c924..b6d12d66d04b 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -92,7 +92,10 @@ int xstateregs_get(struct task_struct *target, const struct
user_regset *regset,
fpu__activate_fpstate_read(fpu);
if (using_compacted_format()) {
- ret = copy_xstate_to_user(pos, count, kbuf, ubuf, xsave);
+ if (kbuf)
+ ret = copy_xstate_to_kernel(pos, count, kbuf, ubuf,
xsave);
+ else
+ ret = copy_xstate_to_user(pos, count, kbuf, ubuf,
xsave);
} else {
fpstate_sanitize_xstate(fpu);
/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index e7bb41723eaa..38561539cb99 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -924,10 +924,106 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
int pkey,
* This is similar to user_regset_copyout(), but will not add offset to
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
-static inline int xstate_copyout(unsigned int pos, unsigned int count,
-void *kbuf, void __user *ubuf,
-const void *data, const int start_pos,
-const int end_pos)
+static inline int
+__copy_xstate_to_kernel(unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf,
+ const void *data, const int start_pos,
+ const int end_pos)
+{
+ if ((count == 0) || (pos < start_pos))
+ return 0;
+
+ if (end_pos < 0 || pos < end_pos) {
+ unsigned int copy = (end_pos < 0 ? count : min(count, end_pos -
pos));
+
+ if (kbuf) {
+ memcpy(kbuf + pos, data, copy);
+ } else {
+ if (__copy_to_user(ubuf + pos, data, copy))
+ return -EFAULT;
+ }
+ }
+ return 0;
+}
+
+/*
+ * Convert from kernel XSAVES compacted format to standard format and copy
+ * to a kernel-space ptrace buffer.
+ *
+ * It supports partial copy but pos always starts from zero. This is called
+ * from xstateregs_get() and there we check the CPU has XSAVES.
+ */
+int copy_xstate_to_kernel(unsigned int pos, unsigned int count, void *kbuf,
+ void __user *ubuf, struct xregs_state *xsave)
+{
+ unsigned int offset, size;
+ int ret, i;
+ struct xstate_header header;
+
+ /*
+* Currently copy_regset_to_user() starts from pos 0:
+*/
+ if (unlikely(pos != 0))
+ return -EFAULT;
+
+ /*
+* The destination is a ptrace buffer; we put in only user xstates:
+*/
+ memset(&header, 0, sizeof(header));
+ header.xfeatures = xsave->header.xfeatures;
+ header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+
+ /*
+* Copy xregs_state->header:
+*/
+ offset = offsetof(struct xregs_state
[PATCH 21/33] x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs
From: Rik van Riel
On Skylake CPUs I noticed that XRSTOR is unable to deal with states
created by copyout_from_xsaves() if the xstate has only SSE/YMM state, and
no FP state. That is, xfeatures had XFEATURE_MASK_SSE set, but not
XFEATURE_MASK_FP.
The reason is that part of the SSE/YMM state lives in the MXCSR and
MXCSR_FLAGS fields of the FP state.
Ensure that whenever we copy SSE or YMM state around, the MXCSR and
MXCSR_FLAGS fields are also copied around.
Signed-off-by: Rik van Riel
Cc: Andy Lutomirski
Cc: Borislav Petkov
Cc: Borislav Petkov
Cc: Dave Hansen
Cc: Fenghua Yu
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Oleg Nesterov
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: Yu-cheng Yu
Link: http://lkml.kernel.org/r/20170210085445.0f1cc...@annuminas.surriel.com
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/fpu/types.h | 3 +++
arch/x86/kernel/fpu/xstate.c | 42
++
2 files changed, 45 insertions(+)
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 0c314a397cf5..71db45ca8870 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -68,6 +68,9 @@ struct fxregs_state {
/* Default value for fxregs_state.mxcsr: */
#define MXCSR_DEFAULT 0x1f80
+/* Copy both mxcsr & mxcsr_flags with a single u64 memcpy: */
+#define MXCSR_AND_FLAGS_SIZE sizeof(u64)
+
/*
* Software based FPU emulation state. This is arbitrary really,
* it matches the x87 format to make it easier to understand:
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0ef35040d0ad..41c52256bdce 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -921,6 +921,23 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int
pkey,
#endif /* ! CONFIG_ARCH_HAS_PKEYS */
/*
+ * Weird legacy quirk: SSE and YMM states store information in the
+ * MXCSR and MXCSR_FLAGS fields of the FP area. That means if the FP
+ * area is marked as unused in the xfeatures header, we need to copy
+ * MXCSR and MXCSR_FLAGS if either SSE or YMM are in use.
+ */
+static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
+{
+ if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
+ return 0;
+
+ if (xfeatures & XFEATURE_MASK_FP)
+ return 0;
+
+ return 1;
+}
+
+/*
* This is similar to user_regset_copyout(), but will not add offset to
* the source data pointer or increment pos, count, kbuf, and ubuf.
*/
@@ -988,6 +1005,12 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state
*xsave, unsigned int of
}
+ if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+ offset = offsetof(struct fxregs_state, mxcsr);
+ size = MXCSR_AND_FLAGS_SIZE;
+ __copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size,
size_total);
+ }
+
/*
* Fill xsave->i387.sw_reserved value for ptrace frame:
*/
@@ -1070,6 +1093,12 @@ int copy_xstate_to_user(void __user *ubuf, struct
xregs_state *xsave, unsigned i
}
+ if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+ offset = offsetof(struct fxregs_state, mxcsr);
+ size = MXCSR_AND_FLAGS_SIZE;
+ __copy_xstate_to_user(ubuf, &xsave->i387.mxcsr, offset, size,
size_total);
+ }
+
/*
* Fill xsave->i387.sw_reserved value for ptrace frame:
*/
@@ -1122,6 +1151,12 @@ int copy_kernel_to_xstate(struct xregs_state *xsave,
const void *kbuf)
}
}
+ if (xfeatures_mxcsr_quirk(xfeatures)) {
+ offset = offsetof(struct fxregs_state, mxcsr);
+ size = MXCSR_AND_FLAGS_SIZE;
+ memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
+ }
+
/*
* The state that came in from userspace was user-state only.
* Mask all the user states out of 'xfeatures':
@@ -1177,6 +1212,13 @@ int copy_user_to_xstate(struct xregs_state *xsave, const
void __user *ubuf)
}
}
+ if (xfeatures_mxcsr_quirk(xfeatures)) {
+ offset = offsetof(struct fxregs_state, mxcsr);
+ size = MXCSR_AND_FLAGS_SIZE;
+ if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size))
+ return -EFAULT;
+ }
+
/*
* The state that came in from userspace was user-state only.
* Mask all the user states out of 'xfeatures':
--
2.11.0
[tip:x86/urgent] objtool: Handle another GCC stack pointer adjustment bug
Commit-ID: 0d0970eef3b03ef08b19da5bc3044410731cf38f
Gitweb: http://git.kernel.org/tip/0d0970eef3b03ef08b19da5bc3044410731cf38f
Author: Josh Poimboeuf
AuthorDate: Wed, 20 Sep 2017 16:24:32 -0500
Committer: Ingo Molnar
CommitDate: Sat, 23 Sep 2017 15:06:19 +0200
objtool: Handle another GCC stack pointer adjustment bug
The kbuild bot reported the following warning with GCC 4.4 and a
randconfig:
net/socket.o: warning: objtool: compat_sock_ioctl()+0x1083: stack state
mismatch: cfa1=7+160 cfa2=-1+0
This is caused by another GCC non-optimization, where it backs up and
restores the stack pointer for no apparent reason:
2f91: 48 89 e0mov%rsp,%rax
2f94: 4c 89 e7mov%r12,%rdi
2f97: 4c 89 f6mov%r14,%rsi
2f9a: ba 20 00 00 00 mov$0x20,%edx
2f9f: 48 89 c4mov%rax,%rsp
This issue would have been happily ignored before the following commit:
dd88a0a0c861 ("objtool: Handle GCC stack pointer adjustment bug")
But now that objtool is paying attention to such stack pointer writes
to/from a register, it needs to understand them properly. In this case
that means recognizing that the "mov %rsp, %rax" instruction is
potentially a backup of the stack pointer.
Reported-by: kbuild test robot
Signed-off-by: Josh Poimboeuf
Cc: Alexander Potapenko
Cc: Andrey Ryabinin
Cc: Andy Lutomirski
Cc: Arnd Bergmann
Cc: Dmitriy Vyukov
Cc: Linus Torvalds
Cc: Matthias Kaehlcke
Cc: Miguel Bernal Marin
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Fixes: dd88a0a0c861 ("objtool: Handle GCC stack pointer adjustment bug")
Link:
http://lkml.kernel.org/r/8c7aa8e9a36fbbb6655d9d8e7cea58958c912da8.1505942196.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar
---
tools/objtool/arch/x86/decode.c | 6 +++---
tools/objtool/check.c | 43 +++--
2 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0e8c8ec..0f22768 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -208,14 +208,14 @@ int arch_decode_instruction(struct elf *elf, struct
section *sec,
break;
case 0x89:
- if (rex == 0x48 && modrm == 0xe5) {
+ if (rex_w && !rex_r && modrm_mod == 3 && modrm_reg == 4) {
- /* mov %rsp, %rbp */
+ /* mov %rsp, reg */
*type = INSN_STACK;
op->src.type = OP_SRC_REG;
op->src.reg = CFI_SP;
op->dest.type = OP_DEST_REG;
- op->dest.reg = CFI_BP;
+ op->dest.reg = op_to_cfi_reg[modrm_rm][rex_b];
break;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f744617..a0c518e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1203,24 +1203,39 @@ static int update_insn_state(struct instruction *insn,
struct insn_state *state)
switch (op->src.type) {
case OP_SRC_REG:
- if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP) {
+ if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
+ cfa->base == CFI_SP &&
+ regs[CFI_BP].base == CFI_CFA &&
+ regs[CFI_BP].offset == -cfa->offset) {
+
+ /* mov %rsp, %rbp */
+ cfa->base = op->dest.reg;
+ state->bp_scratch = false;
+ }
- if (cfa->base == CFI_SP &&
- regs[CFI_BP].base == CFI_CFA &&
- regs[CFI_BP].offset == -cfa->offset) {
+ else if (op->src.reg == CFI_SP &&
+op->dest.reg == CFI_BP && state->drap) {
- /* mov %rsp, %rbp */
- cfa->base = op->dest.reg;
- state->bp_scratch = false;
- }
+ /* drap: mov %rsp, %rbp */
+ regs[CFI_BP].base = CFI_BP;
+ regs[CFI_BP].offset = -state->stack_size;
+ state->bp_scratch = false;
+ }
- else if (state->drap) {
+ else if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
- /* drap: mov %rsp, %rbp */
- regs[CFI_BP].base = CFI_BP;
- regs[CFI_BP].offset =
-state->stack_size;
- state->bp_
[tip:x86/urgent] x86/asm: Fix inline asm call constraints for Clang
Commit-ID: f5caf621ee357279e759c0911daf6d55c7d36f03
Gitweb: http://git.kernel.org/tip/f5caf621ee357279e759c0911daf6d55c7d36f03
Author: Josh Poimboeuf
AuthorDate: Wed, 20 Sep 2017 16:24:33 -0500
Committer: Ingo Molnar
CommitDate: Sat, 23 Sep 2017 15:06:20 +0200
x86/asm: Fix inline asm call constraints for Clang
For inline asm statements which have a CALL instruction, we list the
stack pointer as a constraint to convince GCC to ensure the frame
pointer is set up first:
static inline void foo()
{
register void *__sp asm(_ASM_SP);
asm("call bar" : "+r" (__sp))
}
Unfortunately, that pattern causes Clang to corrupt the stack pointer.
The fix is easy: convert the stack pointer register variable to a global
variable.
It should be noted that the end result is different based on the GCC
version. With GCC 6.4, this patch has exactly the same result as
before:
defconfig defconfig-nofp distro distro-nofp
before 9820389 9491555 8816046 8516940
after 9820389 9491555 8816046 8516940
With GCC 7.2, however, GCC's behavior has changed. It now changes its
behavior based on the conversion of the register variable to a global.
That somehow convinces it to *always* set up the frame pointer before
inserting *any* inline asm. (Therefore, listing the variable as an
output constraint is a no-op and is no longer necessary.) It's a bit
overkill, but the performance impact should be negligible. And in fact,
there's a nice improvement with frame pointers disabled:
defconfig defconfig-nofp distro distro-nofp
before 9796316 9468236 9076191 8790305
after 9796957 9464267 9076381 8785949
So in summary, while listing the stack pointer as an output constraint
is no longer necessary for newer versions of GCC, it's still needed for
older versions.
Suggested-by: Andrey Ryabinin
Reported-by: Matthias Kaehlcke
Signed-off-by: Josh Poimboeuf
Cc: Alexander Potapenko
Cc: Andy Lutomirski
Cc: Arnd Bergmann
Cc: Dmitriy Vyukov
Cc: Linus Torvalds
Cc: Miguel Bernal Marin
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link:
http://lkml.kernel.org/r/3db862e970c432ae823cf515c52b54fec8270e0e.1505942196.git.jpoim...@redhat.com
Signed-off-by: Ingo Molnar
---
arch/x86/include/asm/alternative.h | 3 +--
arch/x86/include/asm/asm.h | 11 +++
arch/x86/include/asm/mshyperv.h | 10 --
arch/x86/include/asm/paravirt_types.h| 14 +++---
arch/x86/include/asm/preempt.h | 15 +--
arch/x86/include/asm/processor.h | 6 ++
arch/x86/include/asm/rwsem.h | 4 ++--
arch/x86/include/asm/uaccess.h | 4 ++--
arch/x86/include/asm/xen/hypercall.h | 5 ++---
arch/x86/kvm/emulate.c | 3 +--
arch/x86/kvm/vmx.c | 3 +--
arch/x86/mm/fault.c | 3 +--
tools/objtool/Documentation/stack-validation.txt | 6 +++---
13 files changed, 42 insertions(+), 45 deletions(-)
diff --git a/arch/x86/include/asm/alternative.h
b/arch/x86/include/asm/alternative.h
index 1b02038..c096624 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -218,10 +218,9 @@ static inline int alternatives_text_reserved(void *start,
void *end)
#define alternative_call_2(oldfunc, newfunc1, feature1, newfunc2, feature2, \
output, input...) \
{\
- register void *__sp asm(_ASM_SP); \
asm volatile (ALTERNATIVE_2("call %P[old]", "call %P[new1]", feature1,\
"call %P[new2]", feature2)\
- : output, "+r" (__sp) \
+ : output, ASM_CALL_CONSTRAINT \
: [old] "i" (oldfunc), [new1] "i" (newfunc1), \
[new2] "i" (newfunc2), ## input); \
}
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 676ee58..c1eadba 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -132,4 +132,15 @@
/* For C file, we already have NOKPROBE_SYMBOL macro */
#endif
+#ifndef __ASSEMBLY__
+/*
+ * This output constraint should be used for any inline asm which has a "call"
+ * instruction. Otherwise the asm may be inserted before the frame pointer
+ * gets set up by the containing function. If you forget to do this, objtool
+ * may print a "call without frame pointer save/setup" warning.
+ */
+register unsigned int __asm_call_sp asm("esp");
+#define ASM_CALL_CONSTRAINT "+r" (__asm_call_sp)
+#endif
+
Re: [PATCHSET for-4.14] cgroup, sched: cgroup2 basic resource accounting
On Fri, Sep 22, 2017 at 11:05:30AM -0700, Tejun Heo wrote: > Peter, ping? Humm,. So I think I was good, but I was under the impression you'd send a new version better explaining the need/design of that iteration stuff. Could be I lost the plot somehow.
Re: [patch 3/3] x86: kvm guest side support for KVM_HC_RT_PRIO hypercall
On Sat, Sep 23, 2017 at 12:56:12PM +0200, Paolo Bonzini wrote: > On 22/09/2017 14:55, Peter Zijlstra wrote: > > You just explained it yourself. If the thread that needs to complete > > what you're waiting on has lower priority, it will _never_ get to run if > > you're busy waiting on it. > > > > This is _trivial_. > > > > And even for !RT it can be quite costly, because you can end up having > > to burn your entire slot of CPU time before you run the other task. > > > > Userspace spinning is _bad_, do not do this. > > This is not userspace spinning, it is guest spinning---which has > effectively the same effect but you cannot quite avoid. So I'm virt illiterate and have no clue on how all this works; but wasn't this a vmexit ? (that's what marcelo traced). And once you've done a vmexit you're a regular task again, not a vcpu. > But I agree that the solution is properly prioritizing threads that can > interrupt the VCPU, and using PI mutexes. Right, if you want to run RT VCPUs the whole emulator/vcpu interaction needs to be designed for RT. > I'm not a priori opposed to paravirt scheduling primitives, but I am not > at all sure that it's required. Problem is that the proposed thing doesn't solve anything. There is nothing that prohibits the guest from triggering a vmexit while holding a spinlock and landing in the self-same problems.
Re: [PATCHSET for-4.14] cgroup, sched: cgroup2 basic resource accounting
Hello, Peter. On Sat, Sep 23, 2017 at 03:37:31PM +0200, Peter Zijlstra wrote: > On Fri, Sep 22, 2017 at 11:05:30AM -0700, Tejun Heo wrote: > > Peter, ping? > > Humm,. So I think I was good, but I was under the impression you'd send > a new version better explaining the need/design of that iteration stuff. > Could be I lost the plot somehow. The updated version was posted quite a while ago. http://lkml.kernel.org/r/20170829174325.gs491...@devbig577.frc2.facebook.com Thanks. -- tejun
Re: [PATCH net-next 2/2] net: dsa: lan9303: Add basic offloading of unicast traffic
> The point is: Once both external ports are in "forwarding", I see no way > to prevent traffic flowing directly between the external ports. Generally, there are port vectors. Port X can send frames only to Port Y. If you don't have that, there are possibilities with VLANs. Each port is given a unique VLAN. All incoming untagged traffic is tagged with the VLAN. You just need to keep the VLAN separated and add/remove the VLAN tag in the dsa tag driver. Andrew
Re: [PATCH 00/33] x86 FPU fixes and cleanups for v4.14
On 23/09/17 15:02, Ingo Molnar wrote: > > * Ingo Molnar wrote: > >> So I'd like to push these changes to Linus tomorrow-ish as an RFC pull >> request in 1-2 days, because there's now 4 fixes depending on these >> changes, and because the result will be more maintainable for the >> LTS v4.14 kernel. >> >> The biggest changes from the earlier iterations is the fixes from >> Eric Biggers for information leaks, plus more cleanups. I have also >> removed the change that Peter Zijlstra and others felt uneasy about, >> the ::last_cpu -> ::fpregs_cached change to the state machine. This >> should make the changes uncontroversial. >> >> Due to taking out that patch I had to rebase the changes, most of which >> have accrued months of testing in linux-next. So I'm pretty confident >> about the overall stability of it. (Famous last words.) > > I forgot to mention that these changes are also available in tip:master > (and tip:WIP.x86/fpu), at: > > git git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git master Really? Did you hijack the Xen tree? :-) Juergen
Re: [media] v4l2-core: Fine-tuning for some function implementations
>> Will the software evolution be continued for related source files? >> Are there any update candidates left over in the directory “v4l2-core”? > > Sorry, I don't understand the question. I try to explain my view again. > We don't want to touch the videobuf-* files unless there is a very good > reason. I hoped that my update suggestions could be good enough once more for this area. > That old videobuf framework is deprecated and the code is quite fragile > (i.e. easy to break things). How do you think about to move this stuff into a separate subdirectory so that it might become a bit easier to distinguish these software components? > Everything else in that directory is under continuous development. I am curious if there are still update candidates left over (also from my selection of change possibilities). Regards, Markus
Re: [PATCH] firmware: dmi_scan: Drop dmi_initialized
Hi Ingo, On Sat, 23 Sep 2017 12:50:31 +0200, Ingo Molnar wrote: > * Jean Delvare wrote: > > > I don't think it makes sense to check for a possible bad > > initialization order at run time on every system when it is all > > decided at build time. > > > > A more efficient way to make sure developers do not introduce new > > calls to dmi_check_system() too early in the initialization sequence > > is to simply document the expected call order. That way, developers > > have a chance to get it right immediately, without having to > > test-boot their kernel, wonder why it does not work, and parse the > > kernel logs for a warning message. And we get rid of the run-time > > performance penalty as a nice side effect. > > Huh? Initialization ordering requirements are very opaque, They were. Now they are very documented. > and by removing the debug check any such bugs are actively hidden. How > is documentation supposed to uncover such bugs once they happen? You are looking at it the wrong way around. Documentation is how they do not happen in the first place. You hit this problem once, 9 years ago. You thought it would have been easier to debug if there was a warning, and you added it. It was one way to solve the problem but I claim it was not the best. What I expect from developers calling a function they aren't familiar with is to read its documentation first. That's the very reason why we spend time writing the documentation. They should not just call the function, boot and see if it works or not. Software engineering vs. trial and error. > So NAK. This was FYI. I maintain this subsystem, and you did not convince me. I also can't see a general trend of implementing what you suggest in the rest of the kernel. Thankfully. -- Jean Delvare SUSE L3 Support
Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early
On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki wrote:
> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki wrote:
>> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
>>> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki
>>> wrote:
>>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain wrote:
>>> >> Ref: https://lkml.org/lkml/2017/9/19/649
>>> >>
>>> >> The bus controllers should suspend the bus operations only after
>>> >> all of the devices on the bus have suspended their device
>>> >> completely. Since the i2c_client drivers could be talking to
>>> >> their devices in their suspend_late() calls, lets ensure that the
>>> >> bus is alive by that time. Thus moving the controller suspend logic to
>>> >> suspend_late().
>>> >>
>>> >> Signed-off-by: Rajat Jain
>>> >> ---
>>> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> index 0e65b97842b4..66dd7f844c40 100644
>>> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>>> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>>> >> .prepare = dw_i2c_plat_prepare,
>>> >> .complete = dw_i2c_plat_complete,
>>> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>>> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend,
>>> >> dw_i2c_plat_resume)
>>> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>>> >>dw_i2c_plat_resume,
>>> >>NULL)
>>> >
>>> > No, you can't just do that.
>>> >
>>> > I sent patches to do it properly before my trip to LA last week, it
>>> > shouldn't be overly difficult to find them in the mailing list
>>> > archives. I can look them up tomorrow if need be.
>>>
>>> Thanks, I am guessing you mean this?
>>>
>>> https://patchwork.kernel.org/patch/9939807/
>>
>> Yes, that's what I mean.
>
> BTW, does this patchset work for you?
Yes, I don't see the issue I was seeing earlier with your patch.
Please feel free to add
Tested-by: Rajat Jain
>
> Thanks,
> Rafael
Re: [PATCH v2] staging: bcm2835-audio: Fix memory corruption
On Sat, Sep 23, 2017 at 12:57:33PM +0200, Stefan Wahren wrote:
> Hi Greg,
>
> > Phil Elwell hat am 11. August 2017 um 12:20
> > geschrieben:
> >
> >
> > The previous commit (0adbfd46) fixed a memory leak but also freed a
> > block in the success case, causing a stale pointer to be used with
> > potentially fatal results. Only free the vchi_instance block in the
> > case that vchi_connect fails; once connected, the instance is
> > retained for subsequent connections.
> >
> > Simplifying the code by removing a bunch of gotos and returning errors
> > directly.
> >
> > Signed-off-by: Phil Elwell
> > Fixes: 0adbfd4694c2 ("staging: bcm2835-audio: fix memory leak in
> > bcm2835_audio_open_connection()")
>
> can you still apply this patch or do you need a resend?
Hm, I don't see this anywhere in my tree, or in my todo mbox, so yes,
please resend it, thanks.
greg k-h
Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early
On Sat, Sep 23, 2017 at 8:49 AM, Rajat Jain wrote:
> On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki wrote:
>> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki
>> wrote:
>>> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki
wrote:
> On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain wrote:
>> Ref: https://lkml.org/lkml/2017/9/19/649
>>
>> The bus controllers should suspend the bus operations only after
>> all of the devices on the bus have suspended their device
>> completely. Since the i2c_client drivers could be talking to
>> their devices in their suspend_late() calls, lets ensure that the
>> bus is alive by that time. Thus moving the controller suspend logic to
>> suspend_late().
>>
>> Signed-off-by: Rajat Jain
>> ---
>> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c
>> b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 0e65b97842b4..66dd7f844c40 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>> .prepare = dw_i2c_plat_prepare,
>> .complete = dw_i2c_plat_complete,
>> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend,
>> dw_i2c_plat_resume)
>> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>>dw_i2c_plat_resume,
>>NULL)
>
> No, you can't just do that.
>
> I sent patches to do it properly before my trip to LA last week, it
> shouldn't be overly difficult to find them in the mailing list
> archives. I can look them up tomorrow if need be.
Thanks, I am guessing you mean this?
https://patchwork.kernel.org/patch/9939807/
>>>
>>> Yes, that's what I mean.
>>
>> BTW, does this patchset work for you?
>
> Yes, I don't see the issue I was seeing earlier with your patch.
> Please feel free to adds,
I think I made my sentence ambiguous. Let me rephrase: Yes, with your
patch, I don't see the issue I was seeing earlier (without your patch)
>
> Tested-by: Rajat Jain
>
>>
>> Thanks,
>> Rafael
[PATCH v2] Asus WMI : Add lightbar led support
Some asus laptops (ROG series for example) are provided
with a lightbar behind the monitor.
This patch make possible to switch it on and off.
This lightbar works exactly as any other led.
V2 : fix whitespaces and typo
Signed-off-by: Maxime Bellengé
---
drivers/platform/x86/asus-wmi.c | 63 +
1 file changed, 63 insertions(+)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 709e3a67391a..6837e7fa52ba 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -119,6 +119,7 @@ MODULE_LICENSE("GPL");
#define ASUS_WMI_DEVID_BRIGHTNESS 0x00050012
#define ASUS_WMI_DEVID_KBD_BACKLIGHT 0x00050021
#define ASUS_WMI_DEVID_LIGHT_SENSOR0x00050022 /* ?? */
+#define ASUS_WMI_DEVID_LIGHTBAR0x00050025
/* Misc */
#define ASUS_WMI_DEVID_CAMERA 0x00060013
@@ -148,6 +149,7 @@ MODULE_LICENSE("GPL");
#define ASUS_WMI_DSTS_BIOS_BIT 0x0004
#define ASUS_WMI_DSTS_BRIGHTNESS_MASK 0x00FF
#define ASUS_WMI_DSTS_MAX_BRIGTH_MASK 0xFF00
+#define ASUS_WMI_DSTS_LIGHTBAR_MASK0x000F
#define ASUS_FAN_DESC "cpu_fan"
#define ASUS_FAN_MFUN 0x13
@@ -222,10 +224,13 @@ struct asus_wmi {
int tpd_led_wk;
struct led_classdev kbd_led;
int kbd_led_wk;
+ struct led_classdev lightbar_led;
+ int lightbar_led_wk;
struct workqueue_struct *led_workqueue;
struct work_struct tpd_led_work;
struct work_struct kbd_led_work;
struct work_struct wlan_led_work;
+ struct work_struct lightbar_led_work;
struct asus_rfkill wlan;
struct asus_rfkill bluetooth;
@@ -567,6 +572,48 @@ static enum led_brightness wlan_led_get(struct
led_classdev *led_cdev)
return result & ASUS_WMI_DSTS_BRIGHTNESS_MASK;
}
+static void lightbar_led_update(struct work_struct *work)
+{
+ int ctrl_param;
+ struct asus_wmi *asus;
+
+ asus = container_of(work, struct asus_wmi, lightbar_led_work);
+
+ ctrl_param = asus->lightbar_led_wk;
+ asus_wmi_set_devstate(ASUS_WMI_DEVID_LIGHTBAR, ctrl_param, NULL);
+}
+
+static void lightbar_led_set(struct led_classdev *led_cdev,
+enum led_brightness value)
+{
+ struct asus_wmi *asus;
+
+ asus = container_of(led_cdev, struct asus_wmi, lightbar_led);
+
+ asus->lightbar_led_wk = !!value;
+ queue_work(asus->led_workqueue, &asus->lightbar_led_work);
+}
+
+static enum led_brightness lightbar_led_get(struct led_classdev *led_cdev)
+{
+ struct asus_wmi *asus;
+ u32 result;
+
+ asus = container_of(led_cdev, struct asus_wmi, lightbar_led);
+ asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_LIGHTBAR, &result);
+
+ return result & ASUS_WMI_DSTS_LIGHTBAR_MASK;
+}
+
+static int lightbar_led_presence(struct asus_wmi *asus)
+{
+ u32 result;
+
+ asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_LIGHTBAR, &result);
+
+ return result & ASUS_WMI_DSTS_PRESENCE_BIT;
+}
+
static void asus_wmi_led_exit(struct asus_wmi *asus)
{
if (!IS_ERR_OR_NULL(asus->kbd_led.dev))
@@ -575,6 +622,8 @@ static void asus_wmi_led_exit(struct asus_wmi *asus)
led_classdev_unregister(&asus->tpd_led);
if (!IS_ERR_OR_NULL(asus->wlan_led.dev))
led_classdev_unregister(&asus->wlan_led);
+ if (!IS_ERR_OR_NULL(asus->lightbar_led.dev))
+ led_classdev_unregister(&asus->lightbar_led);
if (asus->led_workqueue)
destroy_workqueue(asus->led_workqueue);
}
@@ -630,6 +679,20 @@ static int asus_wmi_led_init(struct asus_wmi *asus)
rv = led_classdev_register(&asus->platform_device->dev,
&asus->wlan_led);
+ if (rv)
+ goto error;
+ }
+
+ if (lightbar_led_presence(asus)) {
+ INIT_WORK(&asus->lightbar_led_work, lightbar_led_update);
+
+ asus->lightbar_led.name = "asus::lightbar";
+ asus->lightbar_led.brightness_set = lightbar_led_set;
+ asus->lightbar_led.brightness_get = lightbar_led_get;
+ asus->lightbar_led.max_brightness = 1;
+
+ rv = led_classdev_register(&asus->platform_device->dev,
+ &asus->lightbar_led);
}
error:
--
2.13.5
Re: [PATCH] fix unbalanced page refcounting in bio_map_user_iov
On Fri, Sep 22, 2017 at 01:18:39AM -0400, Vitaly Mayatskikh wrote:
> bio_map_user_iov and bio_unmap_user do unbalanced pages refcounting if
> IO vector has small consecutive buffers belonging to the same page.
> bio_add_pc_page merges them into one, but the page reference is never
> dropped.
>
> Signed-off-by: Vitaly Mayatskikh
>
> diff --git a/block/bio.c b/block/bio.c
> index b38e962fa83e..10cd3b6bed27 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1383,6 +1383,7 @@ struct bio *bio_map_user_iov(struct request_queue *q,
> offset = offset_in_page(uaddr);
> for (j = cur_page; j < page_limit; j++) {
> unsigned int bytes = PAGE_SIZE - offset;
> + unsigned short prev_bi_vcnt = bio->bi_vcnt;
>
> if (len <= 0)
> break;
> @@ -1397,6 +1398,13 @@ struct bio *bio_map_user_iov(struct request_queue *q,
> bytes)
> break;
>
> + /*
> + * check if vector was merged with previous
> + * drop page reference if needed
> + */
> + if (bio->bi_vcnt == prev_bi_vcnt)
> + put_page(pages[j]);
> +
Except that now you've got double-puts on failure exits ;-/
Re: [PATCH] fix unbalanced page refcounting in bio_map_user_iov
On Sat, Sep 23, 2017 at 05:39:28PM +0100, Al Viro wrote:
> On Fri, Sep 22, 2017 at 01:18:39AM -0400, Vitaly Mayatskikh wrote:
> > bio_map_user_iov and bio_unmap_user do unbalanced pages refcounting if
> > IO vector has small consecutive buffers belonging to the same page.
> > bio_add_pc_page merges them into one, but the page reference is never
> > dropped.
> >
> > Signed-off-by: Vitaly Mayatskikh
> >
> > diff --git a/block/bio.c b/block/bio.c
> > index b38e962fa83e..10cd3b6bed27 100644
> > --- a/block/bio.c
> > +++ b/block/bio.c
> > @@ -1383,6 +1383,7 @@ struct bio *bio_map_user_iov(struct request_queue *q,
> > offset = offset_in_page(uaddr);
> > for (j = cur_page; j < page_limit; j++) {
> > unsigned int bytes = PAGE_SIZE - offset;
> > + unsigned short prev_bi_vcnt = bio->bi_vcnt;
> >
> > if (len <= 0)
> > break;
> > @@ -1397,6 +1398,13 @@ struct bio *bio_map_user_iov(struct request_queue *q,
> > bytes)
> > break;
> >
> > + /*
> > +* check if vector was merged with previous
> > +* drop page reference if needed
> > +*/
> > + if (bio->bi_vcnt == prev_bi_vcnt)
> > + put_page(pages[j]);
> > +
>
> Except that now you've got double-puts on failure exits ;-/
IOW, the loop on failure exit should go through the bio, like __bio_unmap_user()
does. We *also* need to put everything left unused in pages[], but only from
the
last iteration through iov_for_each().
Frankly, I would prefer to reuse the pages[], rather than append to it on each
iteration. Used iov_iter_get_pages_alloc(), actually.
Re: [PATCH] coccicheck: improve pattern for getting relative path
Le 10/08/17 à 19:40, Cihangir Akturk a écrit :
When invoked with V=1, coccicheck script prints the information about
which semantic patch (*.cocci file) used for this operation. Actually,
it prints out the relative path of the related semantic patch. The
script uses sed to remove the source tree part from cocci file path like
so:
FILE=`echo $COCCI | sed "s|$srctree/||"`
This pattern works well most of the time. But in cases where $COCCI
doesn't start with "./" characters, it doesn't remove the right part.
Consider the following scenario:
$ make coccicheck COCCI=scripts/coccinelle/api/drm-get-put.cocci \
MODE=patch M=drivers/staging V=1
where
COCCI=scripts/coccinelle/api/drm-get-put.cocci and srctree=.
In this case, out pattern matches the first "s/", and we end up
assigning "scriptcoccinelle/api/drm-get-put.cocci" to $FILE.
Fix this by adding a caret ^ at the beginning of regex pattern, so that
it matches only start of the path.
Signed-off-by: Cihangir Akturk
Acked-by: nicolas.pa...@univ-grenoble-alpes.fr
--- > scripts/coccicheck | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/coccicheck b/scripts/coccicheck
index ec487b8..8de4245 100755
--- a/scripts/coccicheck
+++ b/scripts/coccicheck
@@ -193,7 +193,7 @@ coccinelle () {
if [ $VERBOSE -ne 0 -a $ONLINE -eq 0 ] ; then
- FILE=`echo $COCCI | sed "s|$srctree/||"`
+ FILE=`echo $COCCI | sed "s|^$srctree/||"`
echo "Processing `basename $COCCI`"
echo "with option(s) \"$OPT\""
--
Nicolas Palix
http://lig-membres.imag.fr/palix/
smime.p7s
Description: Signature cryptographique S/MIME
[PATCH] net: dsa: avoid null pointer dereference on p->phy
From: Colin Ian King
Currently p->phy is being null checked in several places to avoid
null pointer dereferences on p->phy, however, the final call
to phy_attached_info on p->phy when p->phy will perform a null
pointer dereference. Fix this by simply moving the call into
the previous code block that is only executed if p->phy is
not null.
Detected by CoverityScan, CID#1457034 ("Dereference after null check")
Fixes: 2220943a21e2 ("phy: Centralise print about attached phy")
Signed-off-by: Colin Ian King
---
net/dsa/slave.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 02ace7d462c4..29ab4e98639b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1115,10 +1115,9 @@ static int dsa_slave_phy_setup(struct net_device
*slave_dev)
of_phy_deregister_fixed_link(port_dn);
return ret;
}
+ phy_attached_info(p->phy);
}
- phy_attached_info(p->phy);
-
return 0;
}
--
2.14.1
Re: [PATCH] net: dsa: avoid null pointer dereference on p->phy
On Sat, 2017-09-23 at 17:57 +0100, Colin King wrote:
> From: Colin Ian King
>
> Currently p->phy is being null checked in several places to avoid
> null pointer dereferences on p->phy, however, the final call
> to phy_attached_info on p->phy when p->phy will perform a null
> pointer dereference. Fix this by simply moving the call into
> the previous code block that is only executed if p->phy is
> not null.
>
> Detected by CoverityScan, CID#1457034 ("Dereference after null check")
>
> Fixes: 2220943a21e2 ("phy: Centralise print about attached phy")
> Signed-off-by: Colin Ian King
> ---
> net/dsa/slave.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 02ace7d462c4..29ab4e98639b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1115,10 +1115,9 @@ static int dsa_slave_phy_setup(struct net_device
> *slave_dev)
> of_phy_deregister_fixed_link(port_dn);
> return ret;
> }
> + phy_attached_info(p->phy);
> }
>
> - phy_attached_info(p->phy);
> -
> return 0;
> }
Huh? Why move this into the test?
The test of the block above this change is
if (!p->phy) {
Perhaps this should be
'
if (p->phy)
phy_attached_info(p->phy);
or simpler
} else {
phy_attached_info(p->phy);
}
or maybe reverse the block
if (p->phy) {
phy_attached_info(p->phy);
} else {
ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
if (ret) {
netdev_err(slave_dev, "failed to connect to port %d:
%d\n",
p->dp->index, ret);
if (phy_is_fixed)
of_phy_deregister_fixed_link(port_dn);
return ret;
}
}
return 0;
}
Re: [PATCH 4/4] tracing: Remove RCU work arounds from stack tracer
On Sat, Sep 23, 2017 at 07:22:04AM -0400, Steven Rostedt wrote: > On Fri, 22 Sep 2017 23:07:37 -0700 > "Paul E. McKenney" wrote: > > > OK, how about the following? > > > > It turns out that functions called from rcu_irq_enter() can > > be subject to various kinds of tracing, which can result in > > rcu_irq_enter() being invoked recursively. This recursion > > causes RCU to not have been watching when it should have, > > resulting in lockdep-RCU splats. Switching from rcu_irq_enter() > > to rcu_nmi_enter() still resulted in failures because of calls > > to rcu_irq_enter() between the rcu_nmi_enter() and its matching > > rcu_nmi_exit(). Such calls again cause RCU to not be watching > > when it should have been. > > > > In particular, the stack tracer called rcu_irq_enter() > > unconditionally, which is problematic when RCU's last > > not-watching-to-watching transition was carried out by > > rcu_nmi_enter(), as will be the case when tracing uses > > rcu_nmi_enter() to cause RCU to start watching the current CPU. > > In that case, rcu_irq_enter() actually switches RCU back to > > the not-watching state for this CPU, which results in lockdep > > splats complaining about rcu_read_lock() being used on an idle > > (not-watched) CPU. The first patch of this series addressed > > this problem by having rcu_irq_enter() and rcu_irq_exit() > > refrain from doing anything when rcu_nmi_enter() caused RCU to > > start watching this CPU. The third patch in this series caused > > save_stack_trace() to invoke rcu_nmi_enter() and rcu_nmi_exit() > > as needed, so this fourth patch now removes the rcu_irq_enter() > > and rcu_irq_exit() from within the stack tracer. > > I think it's a bit too much ;-) I believe it talks too much about the > RCU internals, and the bug will be lost on us mere mortals. > > > > > > Actually, thinking about this more, this doesn't need to go in stable. > > > As recursive rcu_irq_enter() calls should not hurt, and you now allow > > > rcu_irq_enter() to be called even after a rcu_nmi_enter() right? > > > > Yes, it is now the case that rcu_irq_enter() can be called even after > > an rcu_nmi_enter() exited idle, because rcu_irq_enter() now checks for > > this and takes an early exit if so. > > > > But what is it about older kernels prevents the tracing-induced recursive > > calls to rcu_irq_enter()? > > Ah, the bug is if rcu_irq_enter() is called, and the stack trace > triggers then. OK, lets keep it simple, and just say this. > > > Currently the stack tracer calls rcu_irq_enter() to make sure RCU > is watching when it records a stack trace. But if the stack tracer > is triggered while tracing inside of a rcu_irq_enter(), calling > rcu_irq_enter() unconditionally can be problematic. > > The reason for having rcu_irq_enter() in the first place has been > fixed from within the saving of the stack trace code, and there's no > reason for doing it in the stack tracer itself. Just remove it. > > Simple and to the point. Works for me! Thanx, Paul
Re: [PATCH] fix unbalanced page refcounting in bio_map_user_iov
On Sat, Sep 23, 2017 at 05:55:37PM +0100, Al Viro wrote:
> IOW, the loop on failure exit should go through the bio, like
> __bio_unmap_user()
> does. We *also* need to put everything left unused in pages[], but only from
> the
> last iteration through iov_for_each().
>
> Frankly, I would prefer to reuse the pages[], rather than append to it on each
> iteration. Used iov_iter_get_pages_alloc(), actually.
Something like completely untested diff below, perhaps...
diff --git a/block/bio.c b/block/bio.c
index b38e962fa83e..b5fe23597b41 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1323,94 +1323,60 @@ struct bio *bio_map_user_iov(struct request_queue *q,
const struct iov_iter *iter,
gfp_t gfp_mask)
{
- int j;
- int nr_pages = 0;
- struct page **pages;
struct bio *bio;
- int cur_page = 0;
- int ret, offset;
+ struct bio_vec *bvec;
struct iov_iter i;
- struct iovec iov;
-
- iov_for_each(iov, i, *iter) {
- unsigned long uaddr = (unsigned long) iov.iov_base;
- unsigned long len = iov.iov_len;
- unsigned long end = (uaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
- unsigned long start = uaddr >> PAGE_SHIFT;
-
- /*
-* Overflow, abort
-*/
- if (end < start)
- return ERR_PTR(-EINVAL);
-
- nr_pages += end - start;
- /*
-* buffer must be aligned to at least logical block size for now
-*/
- if (uaddr & queue_dma_alignment(q))
- return ERR_PTR(-EINVAL);
- }
+ int ret, j;
- if (!nr_pages)
+ if (!iov_iter_count(iter))
return ERR_PTR(-EINVAL);
- bio = bio_kmalloc(gfp_mask, nr_pages);
+ bio = bio_kmalloc(gfp_mask, iov_iter_npages(iter, BIO_MAX_PAGES));
if (!bio)
return ERR_PTR(-ENOMEM);
- ret = -ENOMEM;
- pages = kcalloc(nr_pages, sizeof(struct page *), gfp_mask);
- if (!pages)
- goto out;
+ i = *iter;
+ while (iov_iter_count(&i)) {
+ struct page **pages;
+ size_t offs;
+ ssize_t bytes;
+ int npages, j;
- iov_for_each(iov, i, *iter) {
- unsigned long uaddr = (unsigned long) iov.iov_base;
- unsigned long len = iov.iov_len;
- unsigned long end = (uaddr + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
- unsigned long start = uaddr >> PAGE_SHIFT;
- const int local_nr_pages = end - start;
- const int page_limit = cur_page + local_nr_pages;
-
- ret = get_user_pages_fast(uaddr, local_nr_pages,
- (iter->type & WRITE) != WRITE,
- &pages[cur_page]);
- if (ret < local_nr_pages) {
- ret = -EFAULT;
+ bytes = iov_iter_get_pages_alloc(&i, &pages, LONG_MAX, &offs);
+ if (bytes <= 0) {
+ ret = bytes ? bytes : -EFAULT;
goto out_unmap;
}
- offset = offset_in_page(uaddr);
- for (j = cur_page; j < page_limit; j++) {
- unsigned int bytes = PAGE_SIZE - offset;
+ npages = DIV_ROUND_UP(offs + bytes, PAGE_SIZE);
- if (len <= 0)
- break;
+ if (unlikely(offs & queue_dma_alignment(q))) {
+ ret = -EINVAL;
+ j = 0;
+ } else {
+ for (j = 0; j < npages; j++) {
+ struct page *page = pages[j];
+ unsigned n = PAGE_SIZE - offs;
+ unsigned prev_bi_vcnt = bio->bi_vcnt;
+
+ if (!bio_add_pc_page(q, bio, page, n, offs)) {
+ iov_iter_truncate(&i, 0);
+ break;
+ }
+
+ if (bio->bi_vcnt == prev_bi_vcnt)
+ put_page(page);
- if (bytes > len)
- bytes = len;
-
- /*
-* sorry...
-*/
- if (bio_add_pc_page(q, bio, pages[j], bytes, offset) <
- bytes)
- break;
-
- len -= bytes;
- offset = 0;
+ iov_iter_advance(&i, n);
+ bytes -= n;
+ offs = 0;
+ }
}
-
- cur
Re: [PATCH] net: dsa: avoid null pointer dereference on p->phy
On 09/23/2017 09:57 AM, Colin King wrote:
> From: Colin Ian King
>
> Currently p->phy is being null checked in several places to avoid
> null pointer dereferences on p->phy, however, the final call
> to phy_attached_info on p->phy when p->phy will perform a null
> pointer dereference. Fix this by simply moving the call into
> the previous code block that is only executed if p->phy is
> not null.
>
> Detected by CoverityScan, CID#1457034 ("Dereference after null check")
The code flow is not exactly easy to read, but I don't see how we can
actually wind up in that situation because we check the return values of
of_phy_connect() and dsa_slave_phy_connect() earlier on.
>
> Fixes: 2220943a21e2 ("phy: Centralise print about attached phy")
> Signed-off-by: Colin Ian King
> ---
> net/dsa/slave.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 02ace7d462c4..29ab4e98639b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1115,10 +1115,9 @@ static int dsa_slave_phy_setup(struct net_device
> *slave_dev)
> of_phy_deregister_fixed_link(port_dn);
> return ret;
> }
> + phy_attached_info(p->phy);
> }
>
> - phy_attached_info(p->phy);
> -
> return 0;
> }
>
>
--
Florian
[PATCH] Documentation: DT bindings: Add chip compatible string for audio codec.
This patch adds chip compatible string to remove the warnings
generated by checkpatch that -DT compatible string
"toshiba,apb-dummy-codec" appears un-documented.
Signed-off-by: Shreeya Patel
---
Documentation/devicetree/bindings/sound/sirf-audio-codec.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/sound/sirf-audio-codec.txt
b/Documentation/devicetree/bindings/sound/sirf-audio-codec.txt
index 062f5ec..a61560b 100644
--- a/Documentation/devicetree/bindings/sound/sirf-audio-codec.txt
+++ b/Documentation/devicetree/bindings/sound/sirf-audio-codec.txt
@@ -3,6 +3,7 @@ SiRF internal audio CODEC
Required properties:
- compatible : "sirf,atlas6-audio-codec" or "sirf,prima2-audio-codec"
+"toshiba,apb-dummy-codec"
- reg : the register address of the device.
@@ -11,7 +12,7 @@ Required properties:
Example:
audiocodec: audiocodec@b004 {
- compatible = "sirf,atlas6-audio-codec";
+ compatible = "sirf,atlas6-audio-codec", "toshiba,apb-dummy-codec";
reg = <0xb004 0x1>;
clocks = <&clks 27>;
};
--
2.7.4
Re: [PATCH] x86/fpu: Simplify fpu__activate_fpstate_read()
On Sat, Sep 23, 2017 at 01:29:32PM +0200, Ingo Molnar wrote: > > > > > > > > Ok - could you please rebase these to to tip:master that is at: > > > > > > > > git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git > > > > master > > > > > > > > In particular this has a WIP.x86/fpu branch with FPU fixes+changes > > > > queued up but > > > > not merged upstream (yet), which conflict with these changes. I'd like > > > > to merge > > > > them all together. > > > > > > > > > > Working on it, but there is a problem with current tip. PTRACE_GETREGSET > > > is > > > causing the following warning: > > > > Yes, the warning should be harmless, and I fixed it locally earlier today - > > does > > the patch below solve it for you as well? > > Note that this fix is now part of tip:master as well, so if you re-test -tip > you > should get all the latest fixes as well (including yours!). > No issues so far with the latest tip/master (commit e7c6e3675331). Eric
[PATCH] irqchip/gic-v3-its: Fix the incorrect BUG_ON in its_init_vpe_domain()
The driver probe path hits 'BUG_ON(entries != vpe_proxy.dev->nr_ites)' on systems where it has VLPI capability, doesn't support direct LPI feature and boot with a single CPU. Relax the BUG_ON() condition to fix the issue. Signed-off-by: Shanker Donthineni --- drivers/irqchip/irq-gic-v3-its.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index e8d8934..e3a59f4 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -2851,7 +2851,7 @@ static int its_init_vpe_domain(void) return -ENOMEM; } - BUG_ON(entries != vpe_proxy.dev->nr_ites); + BUG_ON(entries > vpe_proxy.dev->nr_ites); raw_spin_lock_init(&vpe_proxy.lock); vpe_proxy.next_victim = 0; -- Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
drivers/gpu/drm/nouveau/nvkm/subdev/therm/temp.c: broken emergency poweroff handling
Hi!
Not only it is unneccessarily complex, it is also broken; GFP_ATOMIC
allocation can fail.. and then you fail to shut down the machine.
Someone please fix this.
Thanks,
Pavel
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/temp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/temp.c
@@ -120,6 +120,11 @@ nvkm_therm_sensor_event(struct nvkm_therm *therm, enum
nvkm_therm_thrs thrs,
struct work_struct *work;
work = kmalloc(sizeof(*work), GFP_ATOMIC);
+ /* FIXME:
+ 1) this is total overkill, orderly_poweroff() already
+ uses schedule_work internally
+ 2) it would be good to at least printk what is
going on
+ */
if (work) {
INIT_WORK(work, nv_poweroff_work);
schedule_work(work);
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
[PATCH] omap_ssi_core: fix kilo to be "k" not "K"
Kilo should be "k" not "K", fix it it comments and messages.
Signed-off-by: Pavel Machek
index 88e48b3..41a09f5 100644
--- a/drivers/hsi/controllers/omap_ssi_core.c
+++ b/drivers/hsi/controllers/omap_ssi_core.c
@@ -334,7 +334,7 @@ static int ssi_clk_event(struct notifier_block *nb,
unsigned long event,
case POST_RATE_CHANGE:
dev_dbg(&ssi->device, "post rate change (%lu -> %lu)\n",
clk_data->old_rate, clk_data->new_rate);
- omap_ssi->fck_rate = DIV_ROUND_CLOSEST(clk_data->new_rate,
1000); /* KHz */
+ omap_ssi->fck_rate = DIV_ROUND_CLOSEST(clk_data->new_rate,
1000); /* kHz */
for (i = 0; i < ssi->num_ports; i++) {
omap_port = omap_ssi->port[i];
@@ -467,9 +467,9 @@ static int ssi_hw_init(struct hsi_controller *ssi)
}
/* Resetting GDD */
writel_relaxed(SSI_SWRESET, omap_ssi->gdd + SSI_GDD_GRST_REG);
- /* Get FCK rate in KHz */
+ /* Get FCK rate in kHz */
omap_ssi->fck_rate = DIV_ROUND_CLOSEST(ssi_get_clk_rate(ssi), 1000);
- dev_dbg(&ssi->device, "SSI fck rate %lu KHz\n", omap_ssi->fck_rate);
+ dev_dbg(&ssi->device, "SSI fck rate %lu kHz\n", omap_ssi->fck_rate);
writel_relaxed(SSI_CLK_AUTOGATING_ON, omap_ssi->sys + SSI_GDD_GCR_REG);
omap_ssi->gdd_gcr = SSI_CLK_AUTOGATING_ON;
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
drivers/platform/mips/cpu_hwmon.c: orderly_poweroff() is silent
Orderly poweroff is silent, so it would be nice to
pr_alert("something") at the place of FIXME.
Thanks,
Pavel
diff --git a/drivers/platform/mips/cpu_hwmon.c
b/drivers/platform/mips/cpu_hwmon.c
index 322de58..df24292 100644
--- a/drivers/platform/mips/cpu_hwmon.c
+++ b/drivers/platform/mips/cpu_hwmon.c
@@ -147,6 +147,7 @@ static void do_thermal_timer(struct work_struct *work)
if (temp_max <= CPU_THERMAL_THRESHOLD)
schedule_delayed_work(&thermal_work, msecs_to_jiffies(5000));
else
+ /* FIXME: printk message about what is going on */
orderly_poweroff(true);
}
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
[RESEND] Re: usb/net/p54: trying to register non-static key in p54_unregister_leds
This got rejected by gmail once. Let's see if it works now.
On Thursday, September 21, 2017 8:22:45 PM CEST Andrey Konovalov wrote:
> On Wed, Sep 20, 2017 at 9:55 PM, Johannes Berg
> wrote:
> > On Wed, 2017-09-20 at 21:27 +0200, Christian Lamparter wrote:
> >
> >> It seems this is caused as a result of:
> >> -> lock_map_acquire(&work->lockdep_map);
> >> lock_map_release(&work->lockdep_map);
> >>
> >> in flush_work() [0]
> >
> > Agree.
> >
> >> This was added by:
> >>
> >> commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
> >> Author: Stephen Boyd
> >> Date: Fri Apr 20 17:28:50 2012 -0700
> >>
> >> workqueue: Catch more locking problems with flush_work()
> >
> > Yes, but that doesn't matter.
> >
> >> Looking at the Stephen's patch, it's clear that it was made
> >> with "static DECLARE_WORK(work, my_work)" in mind. However
> >> p54's led_work is "per-device", hence it is stored in the
> >> devices context p54_common, which is dynamically allocated.
> >> So, maybe revert Stephen's patch?
> >
> > I disagree - as the lockdep warning says:
> >
> >> > INFO: trying to register non-static key.
> >> > the code is fine but needs lockdep annotation.
> >> > turning off the locking correctness validator.
> >
> > What it needs is to actually correctly go through initializing the work
> > at least once.
> >
> > Without more information, I can't really say what's going on, but I
> > assume that something is failing and p54_unregister_leds() is getting
> > invoked without p54_init_leds() having been invoked, so essentially
> > it's trying to flush a work that was never initialized?
> >
> > INIT_DELAYED_WORK() does, after all, initialize the lockdep map
> > properly via __INIT_WORK().
Ok, thanks. This does indeed explain it.
But this also begs the question: Is this really working then?
>From what I can tell, if CONFIG_LOCKDEP is not set then there's no BUG
no WARN, no other splat or any other odd system behaviour. Does
[cancel | flush]_[delayed_]work[_sync] really "just work" by *accident*,
as long the delayed_work | work_struct is zeroed out?
And should it work in the future as well?
> Since I'm able to reproduce this, please let me know if you need me to
> collect some debug traces to help with the triage.
Do you want to take a shot at making a patch too? At a quick glance, it
should be enough to move the [#ifdef CONFIG_P54_LEDS ... #endif] block
in p54_unregister_common() into the if (priv->registered) { block
(preferably before the ieee80211_unregister_hw(dev).
Regards,
Christian
[PATCH 0/3] [media] camss-csid: Fine-tuning for three function implementations
From: Markus Elfring Date: Sat, 23 Sep 2017 21:24:56 +0200 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Use common error handling code in csid_set_power() Reduce the scope for a variable in csid_set_power() Adjust a null pointer check in two functions .../media/platform/qcom/camss-8x16/camss-csid.c| 26 +++--- 1 file changed, 13 insertions(+), 13 deletions(-) -- 2.14.1
[PATCH 1/3] [media] camss-csid: Use common error handling code in csid_set_power()
From: Markus Elfring
Date: Sat, 23 Sep 2017 20:48:33 +0200
Add jump targets so that a bit of exception handling can be better reused
at the end of this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring
---
drivers/media/platform/qcom/camss-8x16/camss-csid.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
index 64df82817de3..92d4dc6b4a66 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
@@ -330,13 +330,9 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
ret = csid_set_clock_rates(csid);
- if (ret < 0) {
- regulator_disable(csid->vdda);
- return ret;
- }
+ if (ret < 0)
+ goto disable_regulator;
ret = camss_enable_clocks(csid->nclocks, csid->clock, dev);
- if (ret < 0) {
- regulator_disable(csid->vdda);
- return ret;
- }
+ if (ret < 0)
+ goto disable_regulator;
enable_irq(csid->irq);
@@ -345,8 +341,7 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
if (ret < 0) {
disable_irq(csid->irq);
camss_disable_clocks(csid->nclocks, csid->clock);
- regulator_disable(csid->vdda);
- return ret;
+ goto disable_regulator;
}
hw_version = readl_relaxed(csid->base + CAMSS_CSID_HW_VERSION);
@@ -357,6 +352,11 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
ret = regulator_disable(csid->vdda);
}
+ goto exit;
+
+disable_regulator:
+ regulator_disable(csid->vdda);
+exit:
return ret;
}
--
2.14.1
[PATCH 2/3] [media] camss-csid: Reduce the scope for a variable in csid_set_power()
From: Markus Elfring
Date: Sat, 23 Sep 2017 21:00:30 +0200
Move the definition for the local variable "dev" into an if branch
so that the corresponding setting will only be performed if it was
selected by the parameter "on" of this function.
Signed-off-by: Markus Elfring
---
drivers/media/platform/qcom/camss-8x16/camss-csid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
index 92d4dc6b4a66..ffda0fbfe4d8 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c
@@ -317,10 +317,10 @@ static int csid_reset(struct csid_device *csid)
static int csid_set_power(struct v4l2_subdev *sd, int on)
{
struct csid_device *csid = v4l2_get_subdevdata(sd);
- struct device *dev = to_device_index(csid, csid->id);
int ret;
if (on) {
+ struct device *dev = to_device_index(csid, csid->id);
u32 hw_version;
ret = regulator_enable(csid->vdda);
--
2.14.1
[PATCH 3/3] [media] camss-csid: Adjust a null pointer check in two functions
From: Markus Elfring Date: Sat, 23 Sep 2017 21:10:02 +0200 The script "checkpatch.pl" pointed information out like the following. Comparison to NULL could be written "!format" Thus fix the affected source code places. Signed-off-by: Markus Elfring --- drivers/media/platform/qcom/camss-8x16/camss-csid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/qcom/camss-8x16/camss-csid.c b/drivers/media/platform/qcom/camss-8x16/camss-csid.c index ffda0fbfe4d8..e546f97fa68c 100644 --- a/drivers/media/platform/qcom/camss-8x16/camss-csid.c +++ b/drivers/media/platform/qcom/camss-8x16/camss-csid.c @@ -653,4 +653,4 @@ static int csid_get_format(struct v4l2_subdev *sd, - if (format == NULL) + if (!format) return -EINVAL; fmt->format = *format; @@ -677,4 +677,4 @@ static int csid_set_format(struct v4l2_subdev *sd, - if (format == NULL) + if (!format) return -EINVAL; csid_try_format(csid, cfg, fmt->pad, &fmt->format, fmt->which); -- 2.14.1
[PATCH] Fix C++ kernel in include/linux/mtd/mtd.h
C++ comments look wrong in kernel tree. Fix one.
Signed-off-by: Pavel Machek
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 6cd0f6b..849543f1 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -267,7 +267,7 @@ struct mtd_info {
*/
unsigned int bitflip_threshold;
- // Kernel-only stuff starts here.
+ /* Kernel-only stuff starts here. */
const char *name;
int index;
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
[PATCH] au1k_ir.c fix warning: Prefer [subsystem eg: netdev]_info([subsystem]dev, ...
This patch fixes the following checkpatch.pl warning: fix
Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ...
then pr_info(... to printk(KERN_INFO ...
Signed-off-by: Yurii Pavlenko
---
drivers/staging/irda/drivers/au1k_ir.c | 40 +++---
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/irda/drivers/au1k_ir.c
b/drivers/staging/irda/drivers/au1k_ir.c
index be4ea6a..73e3e4b 100644
--- a/drivers/staging/irda/drivers/au1k_ir.c
+++ b/drivers/staging/irda/drivers/au1k_ir.c
@@ -290,8 +290,7 @@ static int au1k_irda_set_speed(struct net_device *dev, int
speed)
while (irda_read(aup, IR_STATUS) & (IR_RX_STATUS | IR_TX_STATUS)) {
msleep(20);
if (!timeout--) {
- printk(KERN_ERR "%s: rx/tx disable timeout\n",
- dev->name);
+ netdev_err(dev, "rx/tx disable timeout\n");
break;
}
}
@@ -349,7 +348,7 @@ static int au1k_irda_set_speed(struct net_device *dev, int
speed)
IR_RX_ENABLE);
break;
default:
- printk(KERN_ERR "%s unsupported speed %x\n", dev->name, speed);
+ netdev_err(dev, "unsupported speed %x\n", speed);
ret = -EINVAL;
break;
}
@@ -361,18 +360,18 @@ static int au1k_irda_set_speed(struct net_device *dev,
int speed)
irda_write(aup, IR_RING_PROMPT, 0);
if (control & (1 << 14)) {
- printk(KERN_ERR "%s: configuration error\n", dev->name);
+ netdev_err(dev, "configuration error\n");
} else {
if (control & (1 << 11))
- printk(KERN_DEBUG "%s Valid SIR config\n", dev->name);
+ netdev_debug(dev, "Valid SIR config\n");
if (control & (1 << 12))
- printk(KERN_DEBUG "%s Valid MIR config\n", dev->name);
+ netdev_debug(dev, "Valid MIR config\n");
if (control & (1 << 13))
- printk(KERN_DEBUG "%s Valid FIR config\n", dev->name);
+ netdev_debug(dev, "Valid FIR config\n");
if (control & (1 << 10))
- printk(KERN_DEBUG "%s TX enabled\n", dev->name);
+ netdev_debug(dev, "TX enabled\n");
if (control & (1 << 9))
- printk(KERN_DEBUG "%s RX enabled\n", dev->name);
+ netdev_debug(dev, "RX enabled\n");
}
return ret;
@@ -584,23 +583,21 @@ static int au1k_irda_start(struct net_device *dev)
retval = au1k_init(dev);
if (retval) {
- printk(KERN_ERR "%s: error in au1k_init\n", dev->name);
+ netdev_err(dev, "error in au1k_init\n");
return retval;
}
retval = request_irq(aup->irq_tx, &au1k_irda_interrupt, 0,
dev->name, dev);
if (retval) {
- printk(KERN_ERR "%s: unable to get IRQ %d\n",
- dev->name, dev->irq);
+ netdev_err(dev, "unable to get IRQ %d\n", dev->irq);
return retval;
}
retval = request_irq(aup->irq_rx, &au1k_irda_interrupt, 0,
dev->name, dev);
if (retval) {
free_irq(aup->irq_tx, dev);
- printk(KERN_ERR "%s: unable to get IRQ %d\n",
- dev->name, dev->irq);
+ netdev_err(dev, "unable to get IRQ %d\n", dev->irq);
return retval;
}
@@ -673,12 +670,12 @@ static int au1k_irda_hard_xmit(struct sk_buff *skb,
struct net_device *dev)
flags = ptxd->flags;
if (flags & AU_OWN) {
- printk(KERN_DEBUG "%s: tx_full\n", dev->name);
+ netdev_debug(dev, "tx_full\n");
netif_stop_queue(dev);
aup->tx_full = 1;
return 1;
} else if (((aup->tx_head + 1) & (NUM_IR_DESC - 1)) == aup->tx_tail) {
- printk(KERN_DEBUG "%s: tx_full\n", dev->name);
+ netdev_debug(dev, "tx_full\n");
netif_stop_queue(dev);
aup->tx_full = 1;
return 1;
@@ -688,7 +685,7 @@ static int au1k_irda_hard_xmit(struct sk_buff *skb, struct
net_device *dev)
#if 0
if (irda_read(aup, IR_RX_BYTE_CNT) != 0) {
- printk(KERN_DEBUG "tx warning: rx byte cnt %x\n",
+ netdev_debug(dev, "tx warning: rx byte cnt %x\n",
irda_read(aup, IR_RX_BYTE_CNT));
}
#endif
@@ -726,7 +723,7 @@ static void au1k_tx_timeout(struct net_device *dev)
u32 speed;
struct au1k_private *aup = netdev_priv(dev);
- printk(KERN_ERR "%s: tx timeout\n", dev->name);
+ netdev_err(dev, "tx
Re: [PATCH] fix unbalanced page refcounting in bio_map_user_iov
On Sat, Sep 23, 2017 at 06:19:26PM +0100, Al Viro wrote: > On Sat, Sep 23, 2017 at 05:55:37PM +0100, Al Viro wrote: > > > IOW, the loop on failure exit should go through the bio, like > > __bio_unmap_user() > > does. We *also* need to put everything left unused in pages[], but only > > from the > > last iteration through iov_for_each(). > > > > Frankly, I would prefer to reuse the pages[], rather than append to it on > > each > > iteration. Used iov_iter_get_pages_alloc(), actually. > > Something like completely untested diff below, perhaps... > + unsigned n = PAGE_SIZE - offs; > + unsigned prev_bi_vcnt = bio->bi_vcnt; Sorry, that should've been followed by if (n > bytes) n = bytes; Anyway, a carved-up variant is in vfs.git#work.iov_iter. It still needs review and testing; the patch Vitaly has posted in this thread plus 6 followups, hopefully more readable than aggregate diff. Comments?
Re: [PATCH v2 01/31] coccinelle: Improve setup_timer.cocci matching
On Wed, 20 Sep 2017, Kees Cook wrote:
> This improves the patch mode of setup_timer.cocci. Several patterns
> were missing:
> - assignments-before-init_timer() cases
> - limit the .data case removal to the specific struct timer_list instance
> - handling calls by dereference (timer->field vs timer.field)
>
> Cc: Julia Lawall
> Cc: Gilles Muller
> Cc: Nicolas Palix
> Cc: Michal Marek
> Cc: co...@systeme.lip6.fr
> Signed-off-by: Kees Cook
Acked-by: Julia Lawall
Note that I proposed some changes on this rule as well, on August 23
(https://systeme.lip6.fr/pipermail/cocci/2017-August/004386.html). My
changes are still orthogonal to the ones proposed here.
Actually, my changes are in the part about matching, and this patch on
covers the -D patch case (transformation). The matching rules should be
extended in the same way that the patch rules are extended below, but it
would be better to apply my patch first.
julia
> ---
> scripts/coccinelle/api/setup_timer.cocci | 129
> +--
> 1 file changed, 105 insertions(+), 24 deletions(-)
>
> diff --git a/scripts/coccinelle/api/setup_timer.cocci
> b/scripts/coccinelle/api/setup_timer.cocci
> index eb6bd9e4ab1a..279767f3bbef 100644
> --- a/scripts/coccinelle/api/setup_timer.cocci
> +++ b/scripts/coccinelle/api/setup_timer.cocci
> @@ -2,6 +2,7 @@
> /// and data fields
> // Confidence: High
> // Copyright: (C) 2016 Vaishali Thakkar, Oracle. GPLv2
> +// Copyright: (C) 2017 Kees Cook, Google. GPLv2
> // Options: --no-includes --include-headers
> // Keywords: init_timer, setup_timer
>
> @@ -10,60 +11,123 @@ virtual context
> virtual org
> virtual report
>
> +// Match the common cases first to avoid Coccinelle parsing loops with
> +// "... when" clauses.
> +
> @match_immediate_function_data_after_init_timer
> depends on patch && !context && !org && !report@
> expression e, func, da;
> @@
>
> --init_timer (&e);
> -+setup_timer (&e, func, da);
> +-init_timer
> ++setup_timer
> + ( \(&e\|e\)
> ++, func, da
> + );
> +(
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;
> +|
> +-\(e.data\|e->data\) = da;
> +-\(e.function\|e->function\) = func;
> +)
> +
> +@match_immediate_function_data_before_init_timer
> +depends on patch && !context && !org && !report@
> +expression e, func, da;
> +@@
>
> (
> +-\(e.function\|e->function\) = func;
> +-\(e.data\|e->data\) = da;
> +|
> +-\(e.data\|e->data\) = da;
> +-\(e.function\|e->function\) = func;
> +)
> +-init_timer
> ++setup_timer
> + ( \(&e\|e\)
> ++, func, da
> + );
> +
> +@match_function_and_data_after_init_timer
> +depends on patch && !context && !org && !report@
> +expression e, e2, e3, e4, e5, func, da;
> +@@
> +
> +-init_timer
> ++setup_timer
> + ( \(&e\|e\)
> ++, func, da
> + );
> + ... when != func = e2
> + when != da = e3
> +(
> -e.function = func;
> +... when != da = e4
> -e.data = da;
> |
> +-e->function = func;
> +... when != da = e4
> +-e->data = da;
> +|
> -e.data = da;
> +... when != func = e5
> -e.function = func;
> +|
> +-e->data = da;
> +... when != func = e5
> +-e->function = func;
> )
>
> -@match_function_and_data_after_init_timer
> +@match_function_and_data_before_init_timer
> depends on patch && !context && !org && !report@
> -expression e1, e2, e3, e4, e5, a, b;
> +expression e, e2, e3, e4, e5, func, da;
> @@
> -
> --init_timer (&e1);
> -+setup_timer (&e1, a, b);
> -
> -... when != a = e2
> -when != b = e3
> (
> --e1.function = a;
> -... when != b = e4
> --e1.data = b;
> +-e.function = func;
> +... when != da = e4
> +-e.data = da;
> |
> --e1.data = b;
> -... when != a = e5
> --e1.function = a;
> +-e->function = func;
> +... when != da = e4
> +-e->data = da;
> +|
> +-e.data = da;
> +... when != func = e5
> +-e.function = func;
> +|
> +-e->data = da;
> +... when != func = e5
> +-e->function = func;
> )
> +... when != func = e2
> +when != da = e3
> +-init_timer
> ++setup_timer
> + ( \(&e\|e\)
> ++, func, da
> + );
>
> @r1 exists@
> +expression t;
> identifier f;
> position p;
> @@
>
> f(...) { ... when any
> - init_timer@p(...)
> + init_timer@p(\(&t\|t\))
>... when any
> }
>
> @r2 exists@
> +expression r1.t;
> identifier g != r1.f;
> -struct timer_list t;
> expression e8;
> @@
>
> g(...) { ... when any
> - t.data = e8
> + \(t.data\|t->data\) = e8
>... when any
> }
>
> @@ -77,14 +141,31 @@ p << r1.p;
> cocci.include_match(False)
>
> @r3 depends on patch && !context && !org && !report@
> -expression e6, e7, c;
> +expression r1.t, func, e7;
> position r1.p;
> @@
>
> --init_timer@p (&e6);
> -+setup_timer (&e6, c, 0UL);
> -... when != c = e7
> --e6.function = c;
> +(
> +-init_timer@p(&t);
> ++setup_timer(&t, func, 0UL);
> +... when != func = e7
> +-t.function = func;
> +|
> +-t.function = func;
> +... when != func = e7
> +-init_timer@p(&t);
> ++setup_timer(&t, func, 0UL);
> +|
> +-init_timer@p(t);
> ++setup_timer(t, func, 0UL);
> +... when != func = e7
> +-t->function = func;
> +|
> +-t->function = fun
[PATCH 2/2] iio: accel: mma8452: Fix code style warning for unsigned int declarations
Replace 'unsigned' with 'unsigned int'
to improve code readability.
Issue found by checkpatch.
Signed-off-by: Harinath Nampally
---
drivers/iio/accel/mma8452.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index 4a33a26..6194169 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -1088,8 +1088,8 @@ static irqreturn_t mma8452_trigger_handler(int irq, void
*p)
}
static int mma8452_reg_access_dbg(struct iio_dev *indio_dev,
- unsigned reg, unsigned writeval,
- unsigned *readval)
+ unsigned int reg, unsigned int writeval,
+ unsigned int *readval)
{
int ret;
struct mma8452_data *data = iio_priv(indio_dev);
--
2.7.4
[PATCH 1/2] iio: accel: mma8452: Fix code style warning
Replace symbolic permissions with octal permissions to improve code readability. Issue found by checkpatch. Signed-off-by: Harinath Nampally --- drivers/iio/accel/mma8452.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index c352555..4a33a26 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -418,11 +418,11 @@ static ssize_t mma8452_show_os_ratio_avail(struct device *dev, } static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(mma8452_show_samp_freq_avail); -static IIO_DEVICE_ATTR(in_accel_scale_available, S_IRUGO, +static IIO_DEVICE_ATTR(in_accel_scale_available, 0444, mma8452_show_scale_avail, NULL, 0); static IIO_DEVICE_ATTR(in_accel_filter_high_pass_3db_frequency_available, - S_IRUGO, mma8452_show_hp_cutoff_avail, NULL, 0); -static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, S_IRUGO, + 0444, mma8452_show_hp_cutoff_avail, NULL, 0); +static IIO_DEVICE_ATTR(in_accel_oversampling_ratio_available, 0444, mma8452_show_os_ratio_avail, NULL, 0); static int mma8452_get_samp_freq_index(struct mma8452_data *data, -- 2.7.4
[PATCH 0/2] This patchset is for fixes reported by checkpatch.pl
Please find the following patches: iio: accel: mma8452: Fix code style warning for symbolic permissions iio: accel: mma8452: Fix code style warning for unsigned int declarations drivers/iio/accel/mma8452.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.7.4
