* [PATCH 35/66] hwmon: (sht15) use permission-specific DEVICE_ATTR variants
From: Julia Lawall @ 2016-12-22 12:05 UTC (permalink / raw)
To: Jean Delvare; +Cc: kernel-janitors, Guenter Roeck, linux-hwmon, linux-kernel
In-Reply-To: <1482408335-3435-1-git-send-email-Julia.Lawall@lip6.fr>
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.
The semantic patch for the RO case, in the case where the show function
already has the expected name, is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@
DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@
if not (x^"_show" = x_show) then Coccilib.include_match false
@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/hwmon/sht15.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/sht15.c b/drivers/hwmon/sht15.c
index a2fdbb7..f16687c 100644
--- a/drivers/hwmon/sht15.c
+++ b/drivers/hwmon/sht15.c
@@ -769,7 +769,7 @@ static ssize_t sht15_show_humidity(struct device *dev,
return ret ? ret : sprintf(buf, "%d\n", sht15_calc_humid(data));
}
-static ssize_t show_name(struct device *dev,
+static ssize_t name_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
@@ -787,7 +787,7 @@ static SENSOR_DEVICE_ATTR(humidity1_fault, S_IRUGO, sht15_show_status, NULL,
SHT15_STATUS_LOW_BATTERY);
static SENSOR_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR, sht15_show_status,
sht15_store_heater, SHT15_STATUS_HEATER);
-static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+static DEVICE_ATTR_RO(name);
static struct attribute *sht15_attrs[] = {
&sensor_dev_attr_temp1_input.dev_attr.attr,
&sensor_dev_attr_humidity1_input.dev_attr.attr,
^ permalink raw reply related
* [PATCH 18/66] hwmon: (i5k_amb) use permission-specific DEVICE_ATTR variants
From: Julia Lawall @ 2016-12-22 12:04 UTC (permalink / raw)
To: Jean Delvare; +Cc: kernel-janitors, Guenter Roeck, linux-hwmon, linux-kernel
In-Reply-To: <1482408335-3435-1-git-send-email-Julia.Lawall@lip6.fr>
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.
The semantic patch for the RO case, in the case where the show function
already has the expected name, is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@
DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@
if not (x^"_show" = x_show) then Coccilib.include_match false
@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/hwmon/i5k_amb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/i5k_amb.c b/drivers/hwmon/i5k_amb.c
index 6b3d197..a5a9f45 100644
--- a/drivers/hwmon/i5k_amb.c
+++ b/drivers/hwmon/i5k_amb.c
@@ -114,14 +114,14 @@ struct i5k_amb_data {
unsigned int num_attrs;
};
-static ssize_t show_name(struct device *dev, struct device_attribute *devattr,
+static ssize_t name_show(struct device *dev, struct device_attribute *devattr,
char *buf)
{
return sprintf(buf, "%s\n", DRVNAME);
}
-static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+static DEVICE_ATTR_RO(name);
static struct platform_device *amb_pdev;
^ permalink raw reply related
* [PATCH 16/66] hwmon: (core) use permission-specific DEVICE_ATTR variants
From: Julia Lawall @ 2016-12-22 12:04 UTC (permalink / raw)
To: Jean Delvare; +Cc: kernel-janitors, Guenter Roeck, linux-hwmon, linux-kernel
In-Reply-To: <1482408335-3435-1-git-send-email-Julia.Lawall@lip6.fr>
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.
The semantic patch for the RO case, in the case where the show function
already has the expected name, is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@
DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@
if not (x^"_show" = x_show) then Coccilib.include_match false
@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/hwmon/hwmon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 3932f92..0c5660c 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -63,11 +63,11 @@ struct hwmon_thermal_data {
};
static ssize_t
-show_name(struct device *dev, struct device_attribute *attr, char *buf)
+name_show(struct device *dev, struct device_attribute *attr, char *buf)
{
return sprintf(buf, "%s\n", to_hwmon_device(dev)->name);
}
-static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
+static DEVICE_ATTR_RO(name);
static struct attribute *hwmon_dev_attrs[] = {
&dev_attr_name.attr,
^ permalink raw reply related
* [PATCH 00/66] use permission-specific DEVICE_ATTR variants
From: Julia Lawall @ 2016-12-22 12:04 UTC (permalink / raw)
To: linux-kernel; +Cc: kernel-janitors, linux-hwmon, Jean Delvare, Guenter Roeck
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies. A previous version of this semantic patch transformed
only cases where the show and store functions followed particular naming
conventions. This semantic patch additionally renames functions as
needed. It can, however, fail to transform some occurrences where a single
function is used for multiple attributes. In that case, the first case is
transformed an the others are left as is.
The complete semantic patch is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@initialize:ocaml@
@@
let taken = Hashtbl.create 101
// -------------------------------------------------------------------------
// Easy case
@d@
declarer name DEVICE_ATTR;
identifier x;
@@
DEVICE_ATTR(x,...);
@script:ocaml depends on d@
@@
Hashtbl.clear taken
@script:ocaml expected@
x << d.x;
x_show;
x_store;
@@
x_show := make_ident (x^"_show");
x_store := make_ident (x^"_store")
@@
declarer name DEVICE_ATTR_RO;
identifier d.x,expected.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
@@
declarer name DEVICE_ATTR_WO;
identifier d.x,expected.x_store;
@@
- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);
@@
declarer name DEVICE_ATTR_RW;
identifier d.x,expected.x_show,expected.x_store;
@@
- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// -------------------------------------------------------------------------
// Other calls
@o@
declarer name DEVICE_ATTR;
identifier d.x,show,store;
@@
DEVICE_ATTR(x,\(0444\|S_IRUGO\|0200\|S_IWUSR\|0644\|S_IRUGO|S_IWUSR\),
show,store);
@script:ocaml@
x << d.x;
show << o.show;
store << o.store;
@@
if (not(show = "NULL") && Hashtbl.mem taken show) ||
(not(store = "NULL") && Hashtbl.mem taken store)
then Coccilib.include_match false
else (Hashtbl.add taken show (); Hashtbl.add taken store ())
// rename functions
@show1@
identifier o.show,expected.x_show;
parameter list ps;
@@
static
- show(ps)
+ x_show(ps)
{ ... }
@depends on show1@
identifier o.show,expected.x_show;
expression list es;
@@
- show(es)
+ x_show(es)
@depends on show1@
identifier o.show,expected.x_show;
@@
- show
+ x_show
@store1@
identifier o.store,expected.x_store;
parameter list ps;
@@
static
- store(ps)
+ x_store(ps)
{ ... }
@depends on store1@
identifier o.store,expected.x_store;
expression list es;
@@
- store(es)
+ x_store(es)
@depends on store1@
identifier o.store,expected.x_store;
@@
- store
+ x_store
// try again
@@
declarer name DEVICE_ATTR_RO;
identifier d.x,expected.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
@@
declarer name DEVICE_ATTR_WO;
identifier d.x,expected.x_store;
@@
- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);
@@
declarer name DEVICE_ATTR_RW;
identifier d.x,expected.x_show,expected.x_store;
@@
- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// </smpl>
---
drivers/hwmon/adm1021.c | 14 ++--
drivers/hwmon/adm1025.c | 16 ++---
drivers/hwmon/adm1026.c | 128 ++++++++++++++++++++++---------------------
drivers/hwmon/adm1031.c | 15 ++---
drivers/hwmon/adm9240.c | 28 ++++-----
drivers/hwmon/adt7470.c | 48 +++++++---------
drivers/hwmon/adt7475.c | 28 ++++-----
drivers/hwmon/adt7x10.c | 7 +-
drivers/hwmon/asb100.c | 36 ++++++------
drivers/hwmon/atxp1.c | 35 +++++------
drivers/hwmon/dme1737.c | 18 +++---
drivers/hwmon/ds1621.c | 16 ++---
drivers/hwmon/emc2103.c | 36 ++++++------
drivers/hwmon/f71805f.c | 16 ++---
drivers/hwmon/f71882fg.c | 6 +-
drivers/hwmon/fam15h_power.c | 34 +++++------
drivers/hwmon/fschmd.c | 6 +-
drivers/hwmon/g760a.c | 22 +++----
drivers/hwmon/g762.c | 86 ++++++++++++++--------------
drivers/hwmon/gl520sm.c | 48 ++++++++--------
drivers/hwmon/gpio-fan.c | 54 +++++++++---------
drivers/hwmon/hwmon.c | 4 -
drivers/hwmon/i5500_temp.c | 6 +-
drivers/hwmon/i5k_amb.c | 4 -
drivers/hwmon/it87.c | 20 +++---
drivers/hwmon/jz4740-hwmon.c | 6 +-
drivers/hwmon/k10temp.c | 12 ++--
drivers/hwmon/k8temp.c | 4 -
drivers/hwmon/lm63.c | 48 +++++++---------
drivers/hwmon/lm70.c | 6 +-
drivers/hwmon/lm78.c | 38 ++++++------
drivers/hwmon/lm80.c | 4 -
drivers/hwmon/lm83.c | 4 -
drivers/hwmon/lm85.c | 22 +++----
drivers/hwmon/lm87.c | 43 +++++++-------
drivers/hwmon/lm90.c | 8 +-
drivers/hwmon/lm92.c | 10 +--
drivers/hwmon/lm93.c | 39 +++++--------
drivers/hwmon/lm95234.c | 12 ++--
drivers/hwmon/max1111.c | 4 -
drivers/hwmon/max1619.c | 4 -
drivers/hwmon/max197.c | 6 +-
drivers/hwmon/max6650.c | 44 ++++++++------
drivers/hwmon/mc13783-adc.c | 6 +-
drivers/hwmon/mcp3021.c | 6 +-
drivers/hwmon/nct6683.c | 17 ++---
drivers/hwmon/nct6775.c | 4 -
drivers/hwmon/nsa320-hwmon.c | 12 ++--
drivers/hwmon/pc87360.c | 26 ++++----
drivers/hwmon/pc87427.c | 4 -
drivers/hwmon/pcf8591.c | 24 +++-----
drivers/hwmon/sch5627.c | 4 -
drivers/hwmon/sht15.c | 4 -
drivers/hwmon/sis5595.c | 36 ++++++------
drivers/hwmon/smsc47m1.c | 10 +--
drivers/hwmon/smsc47m192.c | 14 ++--
drivers/hwmon/tmp401.c | 11 +--
drivers/hwmon/via-cputemp.c | 6 +-
drivers/hwmon/via686a.c | 8 +-
drivers/hwmon/vt8231.c | 59 ++++++++++---------
drivers/hwmon/w83627ehf.c | 8 +-
drivers/hwmon/w83627hf.c | 53 +++++++++--------
drivers/hwmon/w83781d.c | 34 +++++------
drivers/hwmon/w83791d.c | 23 +++----
drivers/hwmon/w83792d.c | 15 ++---
drivers/hwmon/w83793.c | 6 +-
66 files changed, 709 insertions(+), 726 deletions(-)
^ permalink raw reply
* [PATCH 01/66] hwmon: (adm1021) use permission-specific DEVICE_ATTR variants
From: Julia Lawall @ 2016-12-22 12:04 UTC (permalink / raw)
To: Jean Delvare; +Cc: kernel-janitors, Guenter Roeck, linux-hwmon, linux-kernel
In-Reply-To: <1482408335-3435-1-git-send-email-Julia.Lawall@lip6.fr>
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.
The semantic patch for the RO case, in the case where the show function
already has the expected name, is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@
DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@
if not (x^"_show" = x_show) then Coccilib.include_match false
@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@
- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/hwmon/adm1021.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/hwmon/adm1021.c b/drivers/hwmon/adm1021.c
index 1fdcc3e..eacf10f 100644
--- a/drivers/hwmon/adm1021.c
+++ b/drivers/hwmon/adm1021.c
@@ -191,7 +191,7 @@ static ssize_t show_alarm(struct device *dev, struct device_attribute *attr,
return sprintf(buf, "%u\n", (data->alarms >> index) & 1);
}
-static ssize_t show_alarms(struct device *dev,
+static ssize_t alarms_show(struct device *dev,
struct device_attribute *attr,
char *buf)
{
@@ -251,16 +251,16 @@ static ssize_t set_temp_min(struct device *dev,
return count;
}
-static ssize_t show_low_power(struct device *dev,
+static ssize_t low_power_show(struct device *dev,
struct device_attribute *devattr, char *buf)
{
struct adm1021_data *data = adm1021_update_device(dev);
return sprintf(buf, "%d\n", data->low_power);
}
-static ssize_t set_low_power(struct device *dev,
- struct device_attribute *devattr,
- const char *buf, size_t count)
+static ssize_t low_power_store(struct device *dev,
+ struct device_attribute *devattr,
+ const char *buf, size_t count)
{
struct adm1021_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
@@ -303,8 +303,8 @@ static SENSOR_DEVICE_ATTR(temp2_min, S_IWUSR | S_IRUGO, show_temp_min,
static SENSOR_DEVICE_ATTR(temp2_min_alarm, S_IRUGO, show_alarm, NULL, 3);
static SENSOR_DEVICE_ATTR(temp2_fault, S_IRUGO, show_alarm, NULL, 2);
-static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
-static DEVICE_ATTR(low_power, S_IWUSR | S_IRUGO, show_low_power, set_low_power);
+static DEVICE_ATTR_RO(alarms);
+static DEVICE_ATTR_RW(low_power);
static struct attribute *adm1021_attributes[] = {
&sensor_dev_attr_temp1_max.dev_attr.attr,
^ permalink raw reply related
* Re: [PATCH 2/2] vfio iommu type1: fix the testing of capability for remote task
From: Kirti Wankhede @ 2016-12-22 12:20 UTC (permalink / raw)
To: Jike Song, serge, alex.williamson
Cc: linux-security-module, kvm, linux-kernel, kraxel
In-Reply-To: <1482336616-19252-3-git-send-email-jike.song@intel.com>
On 12/21/2016 9:40 PM, Jike Song wrote:
> Before the mdev enhancement type1 iommu used capable() to test the
> capability of current task; in the course of mdev development a
> new requirement, testing for another task other than current, was
> raised. ns_capable() was used for this purpose, however it still
> tests current, the only difference is, in a specified namespace.
>
> Fix it by using has_capability() instead, which tests the cap for
> specified task in init_user_ns, the same namespace as capable().
>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kirti Wankhede <kwankhede@nvidia.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> ---
> drivers/vfio/vfio_iommu_type1.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index f3726ba..b54aedf 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -394,8 +394,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> long npage, unsigned long *pfn_base)
> {
> unsigned long limit;
> - bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> - CAP_IPC_LOCK);
> + bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
Hi Jike,
Alex's patch already changes this to capable(), you need to resolve.
https://lkml.org/lkml/2016/12/20/490
You need to do only below change, which looks fine to me.
> struct mm_struct *mm;
> long ret, i = 0, lock_acct = 0;
> bool rsvd;
> @@ -491,8 +490,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
> unsigned long *pfn_base, bool do_accounting)
> {
> unsigned long limit;
> - bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> - CAP_IPC_LOCK);
> + bool lock_cap = has_capability(dma->task, CAP_IPC_LOCK);
> struct mm_struct *mm;
> int ret;
> bool rsvd;
>
Thanks,
Kirti
^ permalink raw reply
* [GIT PULL] fs/befs/
From: Luis de Bethencourt @ 2016-12-22 12:18 UTC (permalink / raw)
To: torvalds
Cc: Greg KH, Andrew Morton, Al Viro, Javier Martinez Canillas,
Salah Triki, Theodore Ts'o, Linux Kernel, linux-fsdevel
Hi Linus,
Second git pull request since I took maintainership of the befs file system [0].
A series of small fixes and adding NFS export support.
Thank you very much,
Luis
[0] https://lkml.org/lkml/2016/7/27/502
The following changes since commit 69973b830859bc6529a7a0468ba0d80ee5117826:
Linux 4.9 (2016-12-11 11:17:54 -0800)
are available in the git repository at:
https://github.com/luisbg/linux-befs.git tags/befs-v4.10-rc1
for you to fetch changes up to ac632f5b6301c4beb19f9ea984ce0dc67b6e5874:
befs: add NFS export support (2016-12-22 11:25:24 +0000)
----------------------------------------------------------------
befs fixes for 4.10-rc1
----------------------------------------------------------------
Luis de Bethencourt (9):
befs: fix style issues in debug.c
befs: fix style issues in inode.c
befs: fix style issues in io.c
befs: fix typos in linuxvfs.c
befs: fix style issues in linuxvfs.c
befs: fix style issues in header files
befs: remove signatures from comments
befs: remove trailing whitespaces
befs: add NFS export support
fs/befs/befs.h | 3 +-
fs/befs/befs_fs_types.h | 12 +++---
fs/befs/btree.c | 48 +++++++++++------------
fs/befs/btree.h | 8 ++--
fs/befs/datastream.c | 8 ++--
fs/befs/datastream.h | 5 +--
fs/befs/debug.c | 14 ++++---
fs/befs/inode.c | 12 +++---
fs/befs/inode.h | 5 +--
fs/befs/io.c | 7 ++--
fs/befs/io.h | 1 -
fs/befs/linuxvfs.c | 134 ++++++++++++++++++++++++++++++++++++++++-----------------------
fs/befs/super.h | 4 +-
13 files changed, 145 insertions(+), 116 deletions(-)
^ permalink raw reply
* [v1] mmc: host: dw_mmc-pci:- Handle return NULL error from pcim_iomap_table
From: Arvind Yadav @ 2016-12-22 12:06 UTC (permalink / raw)
To: jh80.chung, ulf.hansson; +Cc: linux-mmc, linux-kernel
Here, If pcim_iomap_table will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/mmc/host/dw_mmc-pci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-pci.c b/drivers/mmc/host/dw_mmc-pci.c
index ab82796..f42d302 100644
--- a/drivers/mmc/host/dw_mmc-pci.c
+++ b/drivers/mmc/host/dw_mmc-pci.c
@@ -61,6 +61,8 @@ static int dw_mci_pci_probe(struct pci_dev *pdev,
return ret;
host->regs = pcim_iomap_table(pdev)[PCI_BAR_NO];
+ if (!host->regs)
+ return -ENOMEM;
pci_set_master(pdev);
--
1.7.9.5
^ permalink raw reply related
* Re: A small window for a race condition in mm/rmap.c:page_lock_anon_vma_read
From: Michal Hocko @ 2016-12-22 12:03 UTC (permalink / raw)
To: Dashi DS1 Cao
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Peter Zijlstra
In-Reply-To: <23B7B563BA4E9446B962B142C86EF24ADBEBB6@CNMAILEX03.lenovo.com>
On Thu 22-12-16 11:53:26, Dashi DS1 Cao wrote:
> I've used another dump with similar backtrace.
Please also dump the anon_vma of the page as well.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* RE: A small window for a race condition in mm/rmap.c:page_lock_anon_vma_read
From: Dashi DS1 Cao @ 2016-12-22 11:53 UTC (permalink / raw)
To: Michal Hocko
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Peter Zijlstra
In-Reply-To: <20161221144343.GD593@dhcp22.suse.cz>
I've used another dump with similar backtrace.
PID: 246 TASK: ffff881fd27df300 CPU: 0 COMMAND: "kswapd0"
#0 [ffff881fcfb23748] machine_kexec at ffffffff81051e9b
#1 [ffff881fcfb237a8] crash_kexec at ffffffff810f27e2
#2 [ffff881fcfb23878] oops_end at ffffffff8163f448
#3 [ffff881fcfb238a0] no_context at ffffffff8162f561
#4 [ffff881fcfb238f0] __bad_area_nosemaphore at ffffffff8162f5f7
#5 [ffff881fcfb23938] bad_area_nosemaphore at ffffffff8162f761
#6 [ffff881fcfb23948] __do_page_fault at ffffffff816421ce
#7 [ffff881fcfb239a8] do_page_fault at ffffffff81642363
#8 [ffff881fcfb239d0] page_fault at ffffffff8163e648
[exception RIP: down_read_trylock+9]
RIP: ffffffff810aa9f9 RSP: ffff881fcfb23a88 RFLAGS: 00010202
RAX: 0000000000000000 RBX: ffff8820833ed940 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000008
RBP: ffff881fcfb23a88 R8: ffffea00779b3a60 R9: ffff883fd0d7b070
R10: 000000000000000e R11: ffffea00049e9580 R12: ffff8820833ed941
R13: ffffea00779b3a40 R14: 0000000000000008 R15: ffffea00779b3a40
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#9 [ffff881fcfb23a90] page_lock_anon_vma_read at ffffffff811a3365
#10 [ffff881fcfb23ac0] page_referenced at ffffffff811a35e7
#11 [ffff881fcfb23b38] shrink_active_list at ffffffff8117e8cc
#12 [ffff881fcfb23bf0] shrink_lruvec at ffffffff8117ef8d
#13 [ffff881fcfb23cf0] shrink_zone at ffffffff8117f2a6
#14 [ffff881fcfb23d48] balance_pgdat at ffffffff8118054c
#15 [ffff881fcfb23e20] kswapd at ffffffff81180813
#16 [ffff881fcfb23ec8] kthread at ffffffff810a5b8f
#17 [ffff881fcfb23f50] ret_from_fork at ffffffff81646a98
crash> print *((struct page *)0xffffea00779b3a40j)
$1 = {
flags = 13510794587668552,
mapping = 0xffff8820833ed941,
{
{
index = 34194823743,
freelist = 0x7f62b9a3f,
pfmemalloc = 63,
pmd_huge_pte = 0x7f62b9a3f
},
{
counters = 8589934592,
{
{
_mapcount = {
counter = 0
},
{
inuse = 0,
objects = 0,
frozen = 0
},
units = 0
},
_count = {
counter = 2
}
}
}
},
{
lru = {
next = 0xdead000000100100,
prev = 0xdead000000200200
},
{
next = 0xdead000000100100,
pages = 2097664,
pobjects = -559087616
},
list = {
next = 0xdead000000100100,
prev = 0xdead000000200200
},
slab_page = 0xdead000000100100
},
{
private = 0,
ptl = {
{
rlock = {
raw_lock = {
{
head_tail = 0,
tickets = {
head = 0,
tail = 0
}
}
}
}
}
},
slab_cache = 0x0,
first_page = 0x0
}
}
crash> disassemble page_lock_anon_vma_read
Dump of assembler code for function page_lock_anon_vma_read:
0xffffffff811a3310 <+0>: nopl 0x0(%rax,%rax,1)
0xffffffff811a3315 <+5>: push %rbp
0xffffffff811a3316 <+6>: mov %rsp,%rbp
0xffffffff811a3319 <+9>: push %r14
0xffffffff811a331b <+11>: push %r13
0xffffffff811a331d <+13>: mov %rdi,%r13
0xffffffff811a3320 <+16>: push %r12
0xffffffff811a3322 <+18>: push %rbx
0xffffffff811a3323 <+19>: mov 0x8(%rdi),%r12
0xffffffff811a3327 <+23>: mov %r12,%rax
0xffffffff811a332a <+26>: and $0x3,%eax
0xffffffff811a332d <+29>: cmp $0x1,%rax
0xffffffff811a3331 <+33>: je 0xffffffff811a3348 <page_lock_anon_vma_read+56>
0xffffffff811a3333 <+35>: xor %ebx,%ebx
0xffffffff811a3335 <+37>: mov %rbx,%rax
0xffffffff811a3338 <+40>: pop %rbx
0xffffffff811a3339 <+41>: pop %r12
0xffffffff811a333b <+43>: pop %r13
0xffffffff811a333d <+45>: pop %r14
0xffffffff811a333f <+47>: pop %rbp
0xffffffff811a3340 <+48>: retq
0xffffffff811a3341 <+49>: nopl 0x0(%rax)
0xffffffff811a3348 <+56>: mov 0x18(%rdi),%eax
0xffffffff811a334b <+59>: test %eax,%eax
0xffffffff811a334d <+61>: js 0xffffffff811a3333 <page_lock_anon_vma_read+35>
0xffffffff811a334f <+63>: mov -0x1(%r12),%r14
0xffffffff811a3354 <+68>: lea -0x1(%r12),%rbx
0xffffffff811a3359 <+73>: add $0x8,%r14
0xffffffff811a335d <+77>: mov %r14,%rdi
0xffffffff811a3360 <+80>: callq 0xffffffff810aa9f0 <down_read_trylock>
0xffffffff811a3365 <+85>: test %eax,%eax
0xffffffff811a3367 <+87>: je 0xffffffff811a3380 <page_lock_anon_vma_read+112>
0xffffffff811a3369 <+89>: mov 0x18(%r13),%eax
0xffffffff811a336d <+93>: test %eax,%eax
0xffffffff811a336f <+95>: jns 0xffffffff811a3335 <page_lock_anon_vma_read+37>
0xffffffff811a3371 <+97>: mov %r14,%rdi
0xffffffff811a3374 <+100>: xor %ebx,%ebx
0xffffffff811a3376 <+102>: callq 0xffffffff810aaa50 <up_read>
0xffffffff811a337b <+107>: jmp 0xffffffff811a3335 <page_lock_anon_vma_read+37>
0xffffffff811a337d <+109>: nopl (%rax)
0xffffffff811a3380 <+112>: mov 0x28(%rbx),%edx
0xffffffff811a3383 <+115>: test %edx,%edx
0xffffffff811a3385 <+117>: je 0xffffffff811a3333 <page_lock_anon_vma_read+35>
0xffffffff811a3387 <+119>: lea 0x1(%rdx),%ecx
0xffffffff811a338a <+122>: lea 0x27(%r12),%rsi
0xffffffff811a338f <+127>: mov %edx,%eax
0xffffffff811a3391 <+129>: lock cmpxchg %ecx,0x27(%r12)
0xffffffff811a3398 <+136>: cmp %edx,%eax
0xffffffff811a339a <+138>: mov %eax,%ecx
0xffffffff811a339c <+140>: jne 0xffffffff811a3402 <page_lock_anon_vma_read+242>
0xffffffff811a339e <+142>: mov 0x18(%r13),%eax
0xffffffff811a33a2 <+146>: test %eax,%eax
0xffffffff811a33a4 <+148>: js 0xffffffff811a33e2 <page_lock_anon_vma_read+210>
0xffffffff811a33a6 <+150>: mov -0x1(%r12),%rax
0xffffffff811a33ab <+155>: lea 0x8(%rax),%rdi
0xffffffff811a33af <+159>: callq 0xffffffff8163ad30 <down_read>
0xffffffff811a33b4 <+164>: lock decl 0x27(%r12)
0xffffffff811a33ba <+170>: sete %al
0xffffffff811a33bd <+173>: test %al,%al
0xffffffff811a33bf <+175>: je 0xffffffff811a3335 <page_lock_anon_vma_read+37>
0xffffffff811a33c5 <+181>: mov -0x1(%r12),%rdi
0xffffffff811a33ca <+186>: add $0x8,%rdi
0xffffffff811a33ce <+190>: callq 0xffffffff810aaa50 <up_read>
0xffffffff811a33d3 <+195>: mov %rbx,%rdi
0xffffffff811a33d6 <+198>: xor %ebx,%ebx
0xffffffff811a33d8 <+200>: callq 0xffffffff811a2dd0 <__put_anon_vma>
0xffffffff811a33dd <+205>: jmpq 0xffffffff811a3335 <page_lock_anon_vma_read+37>
0xffffffff811a33e2 <+210>: lock decl 0x27(%r12)
0xffffffff811a33e8 <+216>: sete %al
0xffffffff811a33eb <+219>: test %al,%al
0xffffffff811a33ed <+221>: je 0xffffffff811a3333 <page_lock_anon_vma_read+35>
0xffffffff811a33f3 <+227>: mov %rbx,%rdi
0xffffffff811a33f6 <+230>: xor %ebx,%ebx
0xffffffff811a33f8 <+232>: callq 0xffffffff811a2dd0 <__put_anon_vma>
0xffffffff811a33fd <+237>: jmpq 0xffffffff811a3335 <page_lock_anon_vma_read+37>
0xffffffff811a3402 <+242>: test %ecx,%ecx
0xffffffff811a3404 <+244>: je 0xffffffff811a3333 <page_lock_anon_vma_read+35>
0xffffffff811a340a <+250>: lea 0x1(%rcx),%edx
0xffffffff811a340d <+253>: mov %ecx,%eax
0xffffffff811a340f <+255>: lock cmpxchg %edx,(%rsi)
0xffffffff811a3413 <+259>: cmp %eax,%ecx
0xffffffff811a3415 <+261>: je 0xffffffff811a339e <page_lock_anon_vma_read+142>
0xffffffff811a3417 <+263>: mov %eax,%ecx
0xffffffff811a3419 <+265>: jmp 0xffffffff811a3402 <page_lock_anon_vma_read+242>
End of assembler dump.
crash>
Dashi Cao
-----Original Message-----
From: Michal Hocko [mailto:mhocko@kernel.org]
Sent: Wednesday, December 21, 2016 10:44 PM
To: Dashi DS1 Cao <caods1@lenovo.com>
Cc: linux-mm@kvack.org; linux-kernel@vger.kernel.org; Peter Zijlstra <peterz@infradead.org>
Subject: Re: A small window for a race condition in mm/rmap.c:page_lock_anon_vma_read
anon_vma locking is clever^Wsubtle as hell. CC Peter...
On Tue 20-12-16 09:32:27, Dashi DS1 Cao wrote:
> I've collected four crash dumps with similar backtrace.
>
> PID: 247 TASK: ffff881fcfad8000 CPU: 14 COMMAND: "kswapd1"
> #0 [ffff881fcfad7978] machine_kexec at ffffffff81051e9b
> #1 [ffff881fcfad79d8] crash_kexec at ffffffff810f27e2
> #2 [ffff881fcfad7aa8] oops_end at ffffffff8163f448
> #3 [ffff881fcfad7ad0] die at ffffffff8101859b
> #4 [ffff881fcfad7b00] do_general_protection at ffffffff8163ed3e
> #5 [ffff881fcfad7b30] general_protection at ffffffff8163e5e8
> [exception RIP: down_read_trylock+9]
> RIP: ffffffff810aa9f9 RSP: ffff881fcfad7be0 RFLAGS: 00010286
> RAX: 0000000000000000 RBX: ffff882b47ddadc0 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000000 RDI:
> 91550b2b32f5a3e8
rdi is obviously a mess - smells like a string. So either sombody has overwritten root_anon_vma or this is really a use after free...
> RBP: ffff881fcfad7be0 R8: ffffea00ecc28860 R9: ffff883fcffeae28
> R10: ffffffff81a691a0 R11: 0000000000000001 R12: ffff882b47ddadc1
> R13: ffffea00ecc28840 R14: 91550b2b32f5a3e8 R15: ffffea00ecc28840
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0000
> #6 [ffff881fcfad7be8] page_lock_anon_vma_read at ffffffff811a3365
> #7 [ffff881fcfad7c18] page_referenced at ffffffff811a35e7
> #8 [ffff881fcfad7c90] shrink_active_list at ffffffff8117e8cc
> #9 [ffff881fcfad7d48] balance_pgdat at ffffffff81180288
> #10 [ffff881fcfad7e20] kswapd at ffffffff81180813
> #11 [ffff881fcfad7ec8] kthread at ffffffff810a5b8f
> #12 [ffff881fcfad7f50] ret_from_fork at ffffffff81646a98
>
> I suspect my customer hits into a small window of a race condition in mm/rmap.c: page_lock_anon_vma_read.
> struct anon_vma *page_lock_anon_vma_read(struct page *page) {
> struct anon_vma *anon_vma = NULL;
> struct anon_vma *root_anon_vma;
> unsigned long anon_mapping;
>
> rcu_read_lock();
> anon_mapping = (unsigned long)READ_ONCE(page->mapping);
> if ((anon_mapping & PAGE_MAPPING_FLAGS) != PAGE_MAPPING_ANON)
> goto out;
> if (!page_mapped(page))
> goto out;
>
> anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
> root_anon_vma = READ_ONCE(anon_vma->root);
Could you dump the anon_vma and struct page as well?
> if (down_read_trylock(&root_anon_vma->rwsem)) {
> /*
> * If the page is still mapped, then this anon_vma is still
> * its anon_vma, and holding the mutex ensures that it will
> * not go away, see anon_vma_free().
> */
> if (!page_mapped(page)) {
> up_read(&root_anon_vma->rwsem);
> anon_vma = NULL;
> }
> goto out;
> }
> ...
> }
>
> Between the time the two "page_mapped(page)" are checked, the address
> (anon_mapping - PAGE_MAPPING_ANON) is unmapped! However it seems that
> anon_vma->root could still be read in but the value is wild. So the
> kernel crashes in down_read_trylock. But it's weird that all the
> "struct page" has its member "_mapcount" still with value 0, not -1,
> in the four crashes.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [v1] misc: cb710: core:- Handle return NULL error from pcim_iomap_table
From: Arvind Yadav @ 2016-12-22 11:58 UTC (permalink / raw)
To: mirq-linux; +Cc: linux-kernel
Here, If pcim_iomap_table will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/misc/cb710/core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/cb710/core.c b/drivers/misc/cb710/core.c
index fb397e7..7b60019 100644
--- a/drivers/misc/cb710/core.c
+++ b/drivers/misc/cb710/core.c
@@ -248,6 +248,8 @@ static int cb710_probe(struct pci_dev *pdev,
spin_lock_init(&chip->irq_lock);
chip->pdev = pdev;
chip->iobase = pcim_iomap_table(pdev)[0];
+ if (!chip->iobase)
+ return -ENOMEM;
pci_set_drvdata(pdev, chip);
--
1.7.9.5
^ permalink raw reply related
* Re: [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: Mika Westerberg @ 2016-12-22 11:50 UTC (permalink / raw)
To: Arvind Yadav
Cc: jarkko.nikula, wsa, andriy.shevchenko, linux-i2c, linux-kernel
In-Reply-To: <1482406759-13094-1-git-send-email-arvind.yadav.cs@gmail.com>
On Thu, Dec 22, 2016 at 05:09:19PM +0530, Arvind Yadav wrote:
> Here, If pcim_iomap_table will fail. It will return NULL.
> Kernel can run into a NULL-pointer dereference.
> This error check will avoid NULL pointer dereference.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index d6423cf..6a1907d 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> dev->controller = controller;
> dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
> dev->base = pcim_iomap_table(pdev)[0];
> + if (!dev->base) {
> + dev_err(&pdev->dev, "I/O map table allocation failed\n");
> + return -ENOMEM;
Are you sure this can actually happen?
IIRC pcim_iomap_regions() (which is called before this) makes sure the
BARs you access here are valid.
^ permalink raw reply
* [v1] i2c: busses: i2c-designware-pcidrv:- Handle return NULL error from pcim_iomap_table
From: Arvind Yadav @ 2016-12-22 11:39 UTC (permalink / raw)
To: jarkko.nikula, wsa
Cc: andriy.shevchenko, mika.westerberg, linux-i2c, linux-kernel
Here, If pcim_iomap_table will fail. It will return NULL.
Kernel can run into a NULL-pointer dereference.
This error check will avoid NULL pointer dereference.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/i2c/busses/i2c-designware-pcidrv.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index d6423cf..6a1907d 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -235,6 +235,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
dev->controller = controller;
dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
dev->base = pcim_iomap_table(pdev)[0];
+ if (!dev->base) {
+ dev_err(&pdev->dev, "I/O map table allocation failed\n");
+ return -ENOMEM;
+ }
dev->dev = &pdev->dev;
dev->irq = pdev->irq;
--
1.7.9.5
^ permalink raw reply related
* RE: [Patch v11 4/5] x86/cpufeature: enable RING3MWAIT for Knights Landing
From: Andrejczuk, Grzegorz @ 2016-12-22 11:37 UTC (permalink / raw)
To: Thomas Gleixner
Cc: mingo@redhat.com, hpa@zytor.com, x86@kernel.org,
linux-kernel@vger.kernel.org, Luc, Piotr,
dave.hansen@linux.intel.com
In-Reply-To: <alpine.DEB.2.20.1612221141450.3426@nanos>
>It also warns on the 64bit build.
It is, I missed it. I changed the type of elf_hwcap2 to long unsigned int.
>> I used set_bit because I wanted to be sure that this operation to be
>> done atomically. There might be data race when multiple values of
>> ELF_HWCAP2 will be set by multiple threads.
>
> Touching ELF_HWCAP2 from anything else than the boot cpu is pointless anyway. This should be done once.
MSR (0x140) is thread specific it has to be set for all physical threads. Also the kernel parameters are handled after boot cpu is initialized and this make disabling harder.
> Aside of that CPU bringup and therefor the call to init_intel() is serialized by the cpu hotplug code and if we lift that, then ELF_HWCAP2 will be the least of our worries.
I do not understand.
^ permalink raw reply
* [GIT PULL] MMC fixes for v.4.10 rc1
From: Ulf Hansson @ 2016-12-22 11:18 UTC (permalink / raw)
To: Linus, linux-kernel@vger.kernel.org, linux-mmc
Cc: Adrian Hunter, Jaehoon Chung, Wolfram Sang
Hi Linus,
Here are some mmc fixes intended for v4.10 rc1. They are based from an
earlier commit during the merge window.
Details are as usual found in the signed tag. Please pull this in!
Merry Christmas!
Ulf Hansson
The following changes since commit e93b1cc8a8965da137ffea0b88e5f62fa1d2a9e6:
Merge branch 'for_linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs
(2016-12-19 08:23:53 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v4.10-3
for you to fetch changes up to e85baa8868b016513c0f5738362402495b1a66a5:
mmc: sd: Meet alignment requirements for raw_ssr DMA (2016-12-21
08:34:30 +0100)
----------------------------------------------------------------
MMC core:
- Further fix thread wake-up for requests
- Use a bounce buffer to fix DMA issue for SSR register read
MMC host:
- sdhci: Fix a regression for runtime PM
- sdhci-cadence: Add a proper SoC specific DT compatible
----------------------------------------------------------------
Adrian Hunter (2):
mmc: sdhci: Fix to handle MMC_POWER_UNDEFINED
mmc: core: Further fix thread wake-up
Masahiro Yamada (1):
mmc: sdhci-cadence: add Socionext UniPhier specific compatible string
Paul Burton (1):
mmc: sd: Meet alignment requirements for raw_ssr DMA
.../devicetree/bindings/mmc/sdhci-cadence.txt | 6 ++--
drivers/mmc/core/core.c | 12 ++++----
drivers/mmc/core/sd.c | 12 ++++++--
drivers/mmc/host/sdhci-cadence.c | 1 +
drivers/mmc/host/sdhci.c | 33 +++++++++++++---------
5 files changed, 39 insertions(+), 25 deletions(-)
^ permalink raw reply
* Re: [PATCH 6/8] blk-mq-sched: add framework for MQ capable IO schedulers
From: Paolo Valente @ 2016-12-22 11:13 UTC (permalink / raw)
To: Paolo Valente; +Cc: Jens Axboe, Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <9936D33B-AE98-46DF-B1DA-D54142A338FD@linaro.org>
> Il giorno 22 dic 2016, alle ore 10:59, Paolo Valente <paolo.valente@linaro.org> ha scritto:
>
>>
>> Il giorno 17 dic 2016, alle ore 01:12, Jens Axboe <axboe@fb.com> ha scritto:
>>
>> This adds a set of hooks that intercepts the blk-mq path of
>> allocating/inserting/issuing/completing requests, allowing
>> us to develop a scheduler within that framework.
>>
>> We reuse the existing elevator scheduler API on the registration
>> side, but augment that with the scheduler flagging support for
>> the blk-mq interfce, and with a separate set of ops hooks for MQ
>> devices.
>>
>> Schedulers can opt in to using shadow requests. Shadow requests
>> are internal requests that the scheduler uses for for the allocate
>> and insert part, which are then mapped to a real driver request
>> at dispatch time. This is needed to separate the device queue depth
>> from the pool of requests that the scheduler has to work with.
>>
>> Signed-off-by: Jens Axboe <axboe@fb.com>
>>
> ...
>
>> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
>> new file mode 100644
>> index 000000000000..b7e1839d4785
>> --- /dev/null
>> +++ b/block/blk-mq-sched.c
>
>> ...
>> +static inline bool
>> +blk_mq_sched_allow_merge(struct request_queue *q, struct request *rq,
>> + struct bio *bio)
>> +{
>> + struct elevator_queue *e = q->elevator;
>> +
>> + if (e && e->type->ops.mq.allow_merge)
>> + return e->type->ops.mq.allow_merge(q, rq, bio);
>> +
>> + return true;
>> +}
>> +
>
> Something does not seem to add up here:
> e->type->ops.mq.allow_merge may be called only in
> blk_mq_sched_allow_merge, which, in its turn, may be called only in
> blk_mq_attempt_merge, which, finally, may be called only in
> blk_mq_merge_queue_io. Yet the latter may be called only if there is
> no elevator (line 1399 and 1507 in blk-mq.c).
>
> Therefore, e->type->ops.mq.allow_merge can never be called, both if
> there is and if there is not an elevator. Be patient if I'm missing
> something huge, but I thought it was worth reporting this.
>
Just another detail: if e->type->ops.mq.allow_merge does get invoked
from the above path, then it is invoked of course without the
scheduler lock held. In contrast, if this function gets invoked
from dd_bio_merge, then the scheduler lock is held.
To handle this opposite alternatives, I don't know whether checking if
the lock is held (and possibly taking it) from inside
e->type->ops.mq.allow_merge is a good solution. In any case, before
possibly trying it, I will wait for some feedback on the main problem,
i.e., on the fact that e->type->ops.mq.allow_merge
seems unreachable in the above path.
Thanks,
Paolo
> Paolo
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [patch 02/10] cpu/hotplug: Prevent overwriting of callbacks
From: Thomas Gleixner @ 2016-12-22 11:07 UTC (permalink / raw)
To: LKML; +Cc: Sebastian Siewior, Ingo Molnar, Peter Zijlstra
In-Reply-To: <20161221192111.675234535@linutronix.de>
On Wed, 21 Dec 2016, Thomas Gleixner wrote:
> Developers manage to overwrite states blindly without thought. That's fatal
> and hard to debug. Add sanity checks to make it fail.
This breaks dynamic states :( Fixed version below.
Thanks,
tglx
8<------------------
Subject: cpu/hotplug: Prevent overwriting of callbacks
From: Thomas Gleixner <tglx@linutronix.de>
Date: Wed, 21 Dec 2016 20:19:49 +0100
Developers manage to overwrite states blindly without thought. That's fatal
and hard to debug. Add sanity checks to make it fail.
This requries to restructure the code so that the dynamic state allocation
happens in the same lock protected section as the actual store. Otherwise
the previous assignment of 'Reserved' to the name field would trigger the
overwrite check.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
---
kernel/cpu.c | 96 ++++++++++++++++++++++++++++++-----------------------------
1 file changed, 50 insertions(+), 46 deletions(-)
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1432,23 +1432,53 @@ static int cpuhp_cb_check(enum cpuhp_sta
return 0;
}
-static void cpuhp_store_callbacks(enum cpuhp_state state,
- const char *name,
- int (*startup)(unsigned int cpu),
- int (*teardown)(unsigned int cpu),
- bool multi_instance)
+/*
+ * Returns a free for dynamic slot assignment of the Online state. The states
+ * are protected by the cpuhp_slot_states mutex and an empty slot is identified
+ * by having no name assigned.
+ */
+static int cpuhp_reserve_state(enum cpuhp_state state)
+{
+ enum cpuhp_state i;
+
+ for (i = CPUHP_AP_ONLINE_DYN; i <= CPUHP_AP_ONLINE_DYN_END; i++) {
+ if (!cpuhp_ap_states[i].name)
+ return i;
+ }
+ WARN(1, "No more dynamic states available for CPU hotplug\n");
+ return -ENOSPC;
+}
+
+static int cpuhp_store_callbacks(enum cpuhp_state state, const char *name,
+ int (*startup)(unsigned int cpu),
+ int (*teardown)(unsigned int cpu),
+ bool multi_instance)
{
/* (Un)Install the callbacks for further cpu hotplug operations */
struct cpuhp_step *sp;
+ int ret = 0;
mutex_lock(&cpuhp_state_mutex);
+
+ if (state == CPUHP_AP_ONLINE_DYN) {
+ ret = cpuhp_reserve_state(state);
+ if (ret < 0)
+ goto out;
+ state = ret;
+ }
sp = cpuhp_get_step(state);
+ if (name && sp->name) {
+ ret = -EBUSY;
+ goto out;
+ }
sp->startup.single = startup;
sp->teardown.single = teardown;
sp->name = name;
sp->multi_instance = multi_instance;
INIT_HLIST_HEAD(&sp->list);
+out:
mutex_unlock(&cpuhp_state_mutex);
+ return ret;
}
static void *cpuhp_get_teardown_cb(enum cpuhp_state state)
@@ -1509,29 +1539,6 @@ static void cpuhp_rollback_install(int f
}
}
-/*
- * Returns a free for dynamic slot assignment of the Online state. The states
- * are protected by the cpuhp_slot_states mutex and an empty slot is identified
- * by having no name assigned.
- */
-static int cpuhp_reserve_state(enum cpuhp_state state)
-{
- enum cpuhp_state i;
-
- mutex_lock(&cpuhp_state_mutex);
- for (i = CPUHP_AP_ONLINE_DYN; i <= CPUHP_AP_ONLINE_DYN_END; i++) {
- if (cpuhp_ap_states[i].name)
- continue;
-
- cpuhp_ap_states[i].name = "Reserved";
- mutex_unlock(&cpuhp_state_mutex);
- return i;
- }
- mutex_unlock(&cpuhp_state_mutex);
- WARN(1, "No more dynamic states available for CPU hotplug\n");
- return -ENOSPC;
-}
-
int __cpuhp_state_add_instance(enum cpuhp_state state, struct hlist_node *node,
bool invoke)
{
@@ -1580,11 +1587,13 @@ EXPORT_SYMBOL_GPL(__cpuhp_state_add_inst
/**
* __cpuhp_setup_state - Setup the callbacks for an hotplug machine state
- * @state: The state to setup
- * @invoke: If true, the startup function is invoked for cpus where
- * cpu state >= @state
- * @startup: startup callback function
- * @teardown: teardown callback function
+ * @state: The state to setup
+ * @invoke: If true, the startup function is invoked for cpus where
+ * cpu state >= @state
+ * @startup: startup callback function
+ * @teardown: teardown callback function
+ * @multi_instance: State is set up for multiple instances which get
+ * added afterwards.
*
* Returns:
* On success:
@@ -1599,25 +1608,16 @@ int __cpuhp_setup_state(enum cpuhp_state
bool multi_instance)
{
int cpu, ret = 0;
- int dyn_state = 0;
if (cpuhp_cb_check(state) || !name)
return -EINVAL;
get_online_cpus();
- /* currently assignments for the ONLINE state are possible */
- if (state == CPUHP_AP_ONLINE_DYN) {
- dyn_state = 1;
- ret = cpuhp_reserve_state(state);
- if (ret < 0)
- goto out;
- state = ret;
- }
+ ret = cpuhp_store_callbacks(state, name, startup, teardown,
+ multi_instance);
- cpuhp_store_callbacks(state, name, startup, teardown, multi_instance);
-
- if (!invoke || !startup)
+ if (ret || !invoke || !startup)
goto out;
/*
@@ -1641,7 +1641,11 @@ int __cpuhp_setup_state(enum cpuhp_state
}
out:
put_online_cpus();
- if (!ret && dyn_state)
+ /*
+ * If the requested state is CPUHP_AP_ONLINE_DYN, return the
+ * dynamically allocated state in case of success.
+ */
+ if (!ret && state == CPUHP_AP_ONLINE_DYN)
return state;
return ret;
}
^ permalink raw reply
* RE: [Patch v11 4/5] x86/cpufeature: enable RING3MWAIT for Knights Landing
From: Thomas Gleixner @ 2016-12-22 11:05 UTC (permalink / raw)
To: Andrejczuk, Grzegorz
Cc: mingo@redhat.com, hpa@zytor.com, x86@kernel.org,
linux-kernel@vger.kernel.org, Luc, Piotr,
dave.hansen@linux.intel.com
In-Reply-To: <ED52C51D9B87F54892CE544909A13C6C1FF9815F@IRSMSX101.ger.corp.intel.com>
On Thu, 22 Dec 2016, Andrejczuk, Grzegorz wrote:
> > Handing a typecasted unsigned int pointer to a function which expects an
> > unsigned long pointer is just broken and a clear sign of careless
> > tinkering.
>
> I thought this to be 32 issue because it popped up in 32 build.
It also warns on the 64bit build.
> The reason for this is probably that sizeof(int) is equal to sizeof(long)
> on x64.
Huch? sizeof(int) is equal to sizeof(long) on 32bit, but definitely not on 64 bit.
> I used the cast following set_cpu_cap define which does exactly the same
> thing with u32* type.
set_cpu_cap() operates on 'c->x86_capability', which is an array of u32 and
the bit numbers are linear. That works because x86 is little endian. It's
not pretty, but it's not a template for general abuse.
> I used set_bit because I wanted to be sure that this operation to be
> done atomically. There might be data race when multiple values of
> ELF_HWCAP2 will be set by multiple threads.
Touching ELF_HWCAP2 from anything else than the boot cpu is pointless
anyway. This should be done once.
Aside of that CPU bringup and therefor the call to init_intel() is
serialized by the cpu hotplug code and if we lift that, then ELF_HWCAP2
will be the least of our worries.
Thanks,
tglx
^ permalink raw reply
* Re: [RFC PATCH v2] crypto: Add IV generation algorithms
From: Binoy Jayan @ 2016-12-22 10:55 UTC (permalink / raw)
To: Herbert Xu
Cc: Milan Broz, Oded, Ofir, David S. Miller, linux-crypto, Mark Brown,
Arnd Bergmann, Linux kernel mailing list, Alasdair Kergon,
Mike Snitzer, dm-devel, Shaohua Li, linux-raid, Rajendra
In-Reply-To: <20161222085509.GA2160@gondor.apana.org.au>
Hi Herbert,
On 22 December 2016 at 14:25, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Dec 13, 2016 at 11:01:08AM +0100, Milan Broz wrote:
>>
>> By the move everything to cryptoAPI we are basically introducing some strange mix
>> of IV and modes there, I wonder how this is going to be maintained.
>> Anyway, Herbert should say if it is ok...
>
> Well there is precedent in how do the IPsec IV generation. In
> that case the IV generators too are completely specific to that
> application, i.e., IPsec. However, the way structured it allowed
> us to have one single entry path from the IPsec stack into the
> crypto layer regardless of whether you are using AEAD or traditional
> encryption/hashing algorithms.
>
> For IPsec we make the IV generators behave like normal AEAD
> algorithms, except that they take the sequence number as the IV.
>
> The goal here are obviously different. However, by employing
> the same method as we do in IPsec, it appears to me that you
> can effectively process multiple blocks at once instead of having
> to supply one block at a time due to the IV generation issue.
Thank you for clarifying that part.:)
So, I hope we can consider algorithms like lmk and tcw too as IV generation
algorithms, even though they manipulate encrypted data directly?
>> I really do not think the disk encryption key management should be moved
>> outside of dm-crypt. We cannot then change key structure later easily.
I agree with this too, only problem with this is when multiple keys are involved
(where the master key is split into 2 or more), and the key selection is made
with a modular division of the (512-byte) sector number by the number of keys.
> It doesn't have to live outside of dm-crypt. You can register
> these IV generators from there if you really want.
Sorry, but I didn't understand this part.
Thanks,
Binoy
^ permalink raw reply
* [PATCH] Fixed status entry in m_can documentation
From: Vyacheslav V. Yurkov @ 2016-12-22 10:45 UTC (permalink / raw)
To: linux-can, netdev, devicetree, linux-kernel, Jiri Kosina
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
Vyacheslav V. Yurkov
Use valid value for 'enabled' in status field
Signed-off-by: Vyacheslav V. Yurkov <uvv.mail@gmail.com>
---
Documentation/devicetree/bindings/net/can/m_can.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/m_can.txt b/Documentation/devicetree/bindings/net/can/m_can.txt
index 9e33177..5facaf5 100644
--- a/Documentation/devicetree/bindings/net/can/m_can.txt
+++ b/Documentation/devicetree/bindings/net/can/m_can.txt
@@ -63,5 +63,5 @@ Board dts:
&m_can1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_m_can1>;
- status = "enabled";
+ status = "okay";
};
--
2.9.0
^ permalink raw reply related
* Re: OOM: Better, but still there on
From: Tetsuo Handa @ 2016-12-22 10:46 UTC (permalink / raw)
To: nholland, mhocko; +Cc: linux-kernel, linux-mm, clm, dsterba, linux-btrfs
In-Reply-To: <20161222103524.GA14020@ppc-nas.fritz.box>
Nils Holland wrote:
> Well, the issue is that I could only do everything via ssh today and
> don't have any physical access to the machines. In fact, both seem to
> have suffered a genuine kernel panic, which is also visible in the
> last few lines of the log I provided today. So, basically, both
> machines are now sitting at my home in panic state and I'll only be
> able to resurrect them wheh I'm physically there again tonight.
# echo 10 > /proc/sys/kernel/panic
^ permalink raw reply
* Re: Patch to include/linux/kernel.h breaks 3rd party modules.
From: Petr Mladek @ 2016-12-22 10:40 UTC (permalink / raw)
To: Valdis Kletnieks; +Cc: Jessica Yu, linux-kernel
In-Reply-To: <30992.1482352925@turing-police.cc.vt.edu>
On Wed 2016-12-21 15:42:05, Valdis Kletnieks wrote:
> Yes, I know that usually out-of-tree modules are on their own.
> However, this one may require a rethink..
>
> (Sorry for not catching this sooner, I hadn't tried to deal with the
> affected module since this patch hit linux-next in next-20161128)
>
> commit 7fd8329ba502ef76dd91db561c7aed696b2c7720
> Author: Petr Mladek <pmladek@suse.com>
> Date: Wed Sep 21 13:47:22 2016 +0200
>
> taint/module: Clean up global and module taint flags handling
>
> Contains this chunk:
>
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -506,6 +506,15 @@ extern enum system_states {
> #define TAINT_UNSIGNED_MODULE 13
> #define TAINT_SOFTLOCKUP 14
> #define TAINT_LIVEPATCH 15
> +#define TAINT_FLAGS_COUNT 16
> +
> +struct taint_flag {
> + char true; /* character printed when tainted */
> + char false; /* character printed when not tainted */
> + bool module; /* also show as a per-module taint flag */
> +};
>
> and hilarity ensues when an out-of-tree module has this:
>
> # ifndef true
> # define true (1)
> # endif
> # ifndef false
> # define false (0)
> # endif
>
> My proposed fix: change true/false to tainted/untainted. If this
> is agreeable, I'll code and submit the fix.
Great catch! I did not have a good feeling about the names. But
I did not found this problem and kept them to reduce changes
in the code.
If we change it, I would go even further and make the purpose
clear, e.g. use char_tainted/char_untainted. The names feel
like booleans wihtout the char_ prefix.
Best Regards,
Petr
^ permalink raw reply
* Re: OOM: Better, but still there on
From: Nils Holland @ 2016-12-22 10:35 UTC (permalink / raw)
To: Michal Hocko
Cc: Tetsuo Handa, linux-kernel, linux-mm, Chris Mason, David Sterba,
linux-btrfs
In-Reply-To: <20161222102725.GG6048@dhcp22.suse.cz>
On Thu, Dec 22, 2016 at 11:27:25AM +0100, Michal Hocko wrote:
> On Thu 22-12-16 11:10:29, Nils Holland wrote:
>
> > However, the log comes from machine #2 again today, as I'm
> > unfortunately forced to try this via VPN from work to home today, so I
> > have exactly one attempt per machine before it goes down and locks up
> > (and I can only restart it later tonight).
>
> This is really surprising to me. Are you sure that you have sysrq
> configured properly. At least sysrq+b shouldn't depend on any memory
> allocations and should allow you to reboot immediately. A sysrq+m right
> before the reboot might turn out being helpful as well.
Well, the issue is that I could only do everything via ssh today and
don't have any physical access to the machines. In fact, both seem to
have suffered a genuine kernel panic, which is also visible in the
last few lines of the log I provided today. So, basically, both
machines are now sitting at my home in panic state and I'll only be
able to resurrect them wheh I'm physically there again tonight. But
that was expected; I could have waited with the test until I'm at
home, which makes things easier, but I thought the sooner I can
provide a log for you to look at, the better. ;-)
Greetings
Nils
^ permalink raw reply
* Re: [PATCH] ib umem: bug: put pid back before return from error path
From: Mark Bloch @ 2016-12-22 8:00 UTC (permalink / raw)
To: Kenneth Lee, dledford, sean.hefty, hal.rosenstock
Cc: robin.murphy, jroedel, egtvedt, vgupta, dave.hansen, lstoakes,
krzk, sebott, linux-rdma, linux-kernel
In-Reply-To: <1482390692-147946-1-git-send-email-liguozhu@hisilicon.com>
Hi,
You have two bugs here:
1) When using ODP, ib_umem_release() checks for umem->odp_data != NULL
calls ib_umem_odp_release() and returns immediately without calling put_pid().
This one isn't in the error path so the title doesn't fit.
2) In case the allocation failed, we return in -ENOMEM without calling put_pid().
Can you please resend this with proper fixes line and a better description of what is going on.
On 22/12/2016 09:11, Kenneth Lee wrote:
> I catched this bug when reading the code. I'm sorry I have no hardware to test
> it. But it is abviously a bug.
>
> Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> ---
> drivers/infiniband/core/umem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 1e62a5f..4609b92 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
>
> if (access & IB_ACCESS_ON_DEMAND) {
> + put_pid(umem->pid);
> ret = ib_umem_odp_get(context, umem);
> if (ret) {
> kfree(umem);
> @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>
> page_list = (struct page **) __get_free_page(GFP_KERNEL);
> if (!page_list) {
> + put_pid(umem->pid);
> kfree(umem);
> return ERR_PTR(-ENOMEM);
> }
>
Mark.
^ permalink raw reply
* Re: [PATCH 0/2] Add further ioctl() operations for namespace discovery
From: Eric W. Biederman @ 2016-12-22 10:28 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Serge E. Hallyn, linux-api, linux-kernel, linux-fsdevel,
Andrey Vagin, James Bottomley, W. Trevor King, Alexander Viro,
Jonathan Corbet, Andrei Vagin
In-Reply-To: <142285b7-225b-fd15-3c8e-9ae2d02e82b5@gmail.com>
"Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:
> Hi Eric,
>
> On 12/22/2016 01:27 AM, Eric W. Biederman wrote:
>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:
>>
>>> Hi Eric,
>>>
>>> On 12/21/2016 01:17 AM, Eric W. Biederman wrote:
>>>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:
>>>>
>>>>> Hi Eric,
>>>>>
>>>>> On 12/20/2016 09:22 PM, Eric W. Biederman wrote:
>>>>>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:
>>>>>>
>>>>>>> Hello Eric,
>>>>>>>
>>>>>>> On 12/19/2016 11:53 PM, Eric W. Biederman wrote:
>>>>>>>> "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com> writes:
>>>>>>>>
>>
>>>> Now the question becomes who are the users of this? Because it just
>>>> occurred to me that we now have an interesting complication. Userspace
>>>> extending the meaning of the capability bits, and using to protect
>>>> additional things. Ugh. That could be a maintenance problem of another
>>>> flavor. Definitely not my favorite.
>>>
>>> I don't follow you here. Could you say some more about what you mean?
>>
>> I have seen user space userspace do thing such as extend CAP_SYS_REBOOT
>> to things such as permission to invoke "shutdown -r now". Which
>> depending on what a clean reboot entails could be greately increasing
>> the scope of CAP_SYS_REBOOT.
>>
>> I am concerned for that and similar situations that userspace
>> applications could lead us into situation that one wrong decision could
>> wind up being an unfixable mistake because fixing the mistake would
>> break userspsace.
>
> Okay.
>
>>>> So why are we asking the questions about what permissions a process has?
>>>
>>> My main interest here is monitoring/discovery/debugging on a running
>>> system. NS_GET_PARENT, NS_GET_USERNS, NS_GET_CREATOR_UID, and
>>> NS_GET_NSTYPE provide most of what I'd like to see. Being able to ask
>>> "does this process have permissions in that namespace?" would be nice
>>> to have in terms of understanding/debugging a system.
>>
>> If we are just looking at explanations then I seem to have been
>> over-engineering things. So let's just aim at the two ioctls.
>> Or at least the information in those ioctls.
>
> Okay.
>
>> With at least a comment on the ioctl returning the OWNER_UID that
>> describes why it is not a problem to if the owners uid is something like
>> ((uid_t)-3). Which overlaps with the space for error return codes.
>>
>> I don't know if we are fine or not, but that review comment definitely
>> deserves some consideration.
>
>
> See my reply just sent to Andrei. We should instead then just return
> the UID via a buffer pointed to by the ioctl() argument:
>
> ioctl(fd, NS_GET_OWNER_UID, &uid);
That will work without problem. Especially as unsigned int is the same
on both 32bit and 64bit so we won't need a compat ioctl.
Eric
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox