[PATCH v2] netfilter: xt_hashlimit: Fix integer divide round to zero.
Diving the divider by the multiplier before applying to the input.
When this would "divide by zero", divide the multiplier by the divider
first then multiply the input by this value.
Currently user2creds outputs zero when input value is bigger than the
number of slices and lower than scale.
This as then user input is applied an integer divide operation to
a number greater than itself (scale).
That rounds up to zero, then we mulitply zero by the credits slice size.
iptables -t filter -I INPUT --protocol tcp --match hashlimit
--hashlimit 40/second --hashlimit-burst 20 --hashlimit-mode srcip
--hashlimit-name syn-flood --jump RETURN
thus trigger the overflow detection code:
xt_hashlimit: overflow, try lower: 25000/20
(25000 as hashlimit avd and 20 the burst)
Here:
134217 slices of (HZ * CREDITS_PER_JIFFY) size.
50 is user input value
100 is XT_HASHLIMIT_SCALE_v2
gives: 0 as user2creds output
Setting burst to "1" typically solve the issue ...
but setting it to "40" does too !
This is on 32bit arch calling into revision 2 of hashlimit.
Signed-off-by: Alban Browaeys <alban.browa...@gmail.com>
---
v2:
- fix missing conditional statement in revision 1 case
.
I removed the code duplication between revision 1 and 2.
v1: https://lkml.org/lkml/2017/2/4/173
---
net/netfilter/xt_hashlimit.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10063408141d..84ad5ab34558 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -463,23 +463,16 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
/* Precision saver. */
static u64 user2credits(u64 user, int revision)
{
- if (revision == 1) {
- /* If multiplying would overflow... */
- if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
- /* Divide first. */
- return div64_u64(user, XT_HASHLIMIT_SCALE)
- * HZ * CREDITS_PER_JIFFY_v1;
-
- return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
-XT_HASHLIMIT_SCALE);
- } else {
- if (user > 0xULL / (HZ*CREDITS_PER_JIFFY))
- return div64_u64(user, XT_HASHLIMIT_SCALE_v2)
- * HZ * CREDITS_PER_JIFFY;
+ u64 scale = (revision == 1) ?
+ XT_HASHLIMIT_SCALE : XT_HASHLIMIT_SCALE_v2;
+ u64 cpj = (revision == 1) ?
+ CREDITS_PER_JIFFY_v1 : CREDITS_PER_JIFFY;
- return div64_u64(user * HZ * CREDITS_PER_JIFFY,
-XT_HASHLIMIT_SCALE_v2);
- }
+ /* Avoid overflow: divide the constant operands first */
+ if (scale >= HZ * cpj)
+ return div64_u64(user, div64_u64(scale, HZ * cpj));
+
+ return user * div64_u64(HZ * cpj, scale);
}
static u32 user2credits_byte(u32 user)
--
2.11.0
[PATCH v2] netfilter: xt_hashlimit: Fix integer divide round to zero.
Diving the divider by the multiplier before applying to the input.
When this would "divide by zero", divide the multiplier by the divider
first then multiply the input by this value.
Currently user2creds outputs zero when input value is bigger than the
number of slices and lower than scale.
This as then user input is applied an integer divide operation to
a number greater than itself (scale).
That rounds up to zero, then we mulitply zero by the credits slice size.
iptables -t filter -I INPUT --protocol tcp --match hashlimit
--hashlimit 40/second --hashlimit-burst 20 --hashlimit-mode srcip
--hashlimit-name syn-flood --jump RETURN
thus trigger the overflow detection code:
xt_hashlimit: overflow, try lower: 25000/20
(25000 as hashlimit avd and 20 the burst)
Here:
134217 slices of (HZ * CREDITS_PER_JIFFY) size.
50 is user input value
100 is XT_HASHLIMIT_SCALE_v2
gives: 0 as user2creds output
Setting burst to "1" typically solve the issue ...
but setting it to "40" does too !
This is on 32bit arch calling into revision 2 of hashlimit.
Signed-off-by: Alban Browaeys
---
v2:
- fix missing conditional statement in revision 1 case
.
I removed the code duplication between revision 1 and 2.
v1: https://lkml.org/lkml/2017/2/4/173
---
net/netfilter/xt_hashlimit.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10063408141d..84ad5ab34558 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -463,23 +463,16 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
/* Precision saver. */
static u64 user2credits(u64 user, int revision)
{
- if (revision == 1) {
- /* If multiplying would overflow... */
- if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
- /* Divide first. */
- return div64_u64(user, XT_HASHLIMIT_SCALE)
- * HZ * CREDITS_PER_JIFFY_v1;
-
- return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
-XT_HASHLIMIT_SCALE);
- } else {
- if (user > 0xULL / (HZ*CREDITS_PER_JIFFY))
- return div64_u64(user, XT_HASHLIMIT_SCALE_v2)
- * HZ * CREDITS_PER_JIFFY;
+ u64 scale = (revision == 1) ?
+ XT_HASHLIMIT_SCALE : XT_HASHLIMIT_SCALE_v2;
+ u64 cpj = (revision == 1) ?
+ CREDITS_PER_JIFFY_v1 : CREDITS_PER_JIFFY;
- return div64_u64(user * HZ * CREDITS_PER_JIFFY,
-XT_HASHLIMIT_SCALE_v2);
- }
+ /* Avoid overflow: divide the constant operands first */
+ if (scale >= HZ * cpj)
+ return div64_u64(user, div64_u64(scale, HZ * cpj));
+
+ return user * div64_u64(HZ * cpj, scale);
}
static u32 user2credits_byte(u32 user)
--
2.11.0
Re: [PATCH] netfilter: xt_hashlimit: Fix integer divide round to zero.
Le lundi 06 février 2017 à 14:04 +0100, Pablo Neira Ayuso a écrit :
> On Sat, Feb 04, 2017 at 11:47:31PM +0100, Alban Browaeys wrote:
> > diff --git a/net/netfilter/xt_hashlimit.c
> > b/net/netfilter/xt_hashlimit.c
> > index 10063408141d..df75ad643eef 100644
> > --- a/net/netfilter/xt_hashlimit.c
> > +++ b/net/netfilter/xt_hashlimit.c
> > @@ -463,21 +463,19 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
> > /* Precision saver. */
> > static u64 user2credits(u64 user, int revision)
> > {
> > > > + /* Avoid overflow: divide the constant operands first */
> > > > if (revision == 1) {
> > > > - /* If multiplying would overflow... */
> > > > - if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
> > > > - /* Divide first. */
> > > > - return div64_u64(user, XT_HASHLIMIT_SCALE)
> > > > - * HZ * CREDITS_PER_JIFFY_v1;
> > > > + return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE,
> > > > + HZ * CREDITS_PER_JIFFY_v1));
> >
> > > > - return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
> > > > + return user * div64_u64(HZ * CREDITS_PER_JIFFY_v1,
> > XT_HASHLIMIT_SCALE);
>
> I see two return statements here, the one coming later is
> shadowed, this can't be right.
I fixed revision 2 case then copy the code to revision 1
and lost the condition in the process.
The code duplication is useless. I will rework it in v2.
Thank you for the review.
Re: [PATCH] netfilter: xt_hashlimit: Fix integer divide round to zero.
Le lundi 06 février 2017 à 14:04 +0100, Pablo Neira Ayuso a écrit :
> On Sat, Feb 04, 2017 at 11:47:31PM +0100, Alban Browaeys wrote:
> > diff --git a/net/netfilter/xt_hashlimit.c
> > b/net/netfilter/xt_hashlimit.c
> > index 10063408141d..df75ad643eef 100644
> > --- a/net/netfilter/xt_hashlimit.c
> > +++ b/net/netfilter/xt_hashlimit.c
> > @@ -463,21 +463,19 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
> > /* Precision saver. */
> > static u64 user2credits(u64 user, int revision)
> > {
> > > > + /* Avoid overflow: divide the constant operands first */
> > > > if (revision == 1) {
> > > > - /* If multiplying would overflow... */
> > > > - if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
> > > > - /* Divide first. */
> > > > - return div64_u64(user, XT_HASHLIMIT_SCALE)
> > > > - * HZ * CREDITS_PER_JIFFY_v1;
> > > > + return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE,
> > > > + HZ * CREDITS_PER_JIFFY_v1));
> >
> > > > - return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
> > > > + return user * div64_u64(HZ * CREDITS_PER_JIFFY_v1,
> > XT_HASHLIMIT_SCALE);
>
> I see two return statements here, the one coming later is
> shadowed, this can't be right.
I fixed revision 2 case then copy the code to revision 1
and lost the condition in the process.
The code duplication is useless. I will rework it in v2.
Thank you for the review.
[PATCH] netfilter: xt_hashlimit: Fix integer divide round to zero.
Diving the divider by the multiplier before applying to the input.
When this would "divide by zero", divide the multiplier by the divider
first then multiply the input by this value.
Currently user2creds outputs zero when input value is bigger than the
number of slices and lower than scale.
This as then user input is applied an integer divide operation to
a number greater than itself (scale).
That rounds up to zero, then we mulitply zero by the credits slice size.
iptables -t filter -I INPUT --protocol tcp --match hashlimit
--hashlimit 40/second --hashlimit-burst 20 --hashlimit-mode srcip
--hashlimit-name syn-flood --jump RETURN
thus trigger the overflow detection code:
xt_hashlimit: overflow, try lower: 25000/20
(25000 as hashlimit avd and 20 the burst)
Here:
134217 slices of (HZ * CREDITS_PER_JIFFY) size.
50 is user input value
100 is XT_HASHLIMIT_SCALE_v2
gives: 0 as user2creds output
Setting burst to "1" typically solve the issue ...
but setting it to "40" does too !
This is on 32bit arch calling into revision 2 of hashlimit.
Signed-off-by: Alban Browaeys <alban.browa...@gmail.com>
---
net/netfilter/xt_hashlimit.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10063408141d..df75ad643eef 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -463,21 +463,19 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
/* Precision saver. */
static u64 user2credits(u64 user, int revision)
{
+ /* Avoid overflow: divide the constant operands first */
if (revision == 1) {
- /* If multiplying would overflow... */
- if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
- /* Divide first. */
- return div64_u64(user, XT_HASHLIMIT_SCALE)
- * HZ * CREDITS_PER_JIFFY_v1;
+ return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE,
+ HZ * CREDITS_PER_JIFFY_v1));
- return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
+ return user * div64_u64(HZ * CREDITS_PER_JIFFY_v1,
XT_HASHLIMIT_SCALE);
} else {
- if (user > 0xULL / (HZ*CREDITS_PER_JIFFY))
- return div64_u64(user, XT_HASHLIMIT_SCALE_v2)
- * HZ * CREDITS_PER_JIFFY;
+ if (XT_HASHLIMIT_SCALE_v2 >= HZ * CREDITS_PER_JIFFY)
+ return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE_v2,
+ HZ * CREDITS_PER_JIFFY));
- return div64_u64(user * HZ * CREDITS_PER_JIFFY,
+ return user * div64_u64(HZ * CREDITS_PER_JIFFY,
XT_HASHLIMIT_SCALE_v2);
}
}
--
2.11.0
[PATCH] netfilter: xt_hashlimit: Fix integer divide round to zero.
Diving the divider by the multiplier before applying to the input.
When this would "divide by zero", divide the multiplier by the divider
first then multiply the input by this value.
Currently user2creds outputs zero when input value is bigger than the
number of slices and lower than scale.
This as then user input is applied an integer divide operation to
a number greater than itself (scale).
That rounds up to zero, then we mulitply zero by the credits slice size.
iptables -t filter -I INPUT --protocol tcp --match hashlimit
--hashlimit 40/second --hashlimit-burst 20 --hashlimit-mode srcip
--hashlimit-name syn-flood --jump RETURN
thus trigger the overflow detection code:
xt_hashlimit: overflow, try lower: 25000/20
(25000 as hashlimit avd and 20 the burst)
Here:
134217 slices of (HZ * CREDITS_PER_JIFFY) size.
50 is user input value
100 is XT_HASHLIMIT_SCALE_v2
gives: 0 as user2creds output
Setting burst to "1" typically solve the issue ...
but setting it to "40" does too !
This is on 32bit arch calling into revision 2 of hashlimit.
Signed-off-by: Alban Browaeys
---
net/netfilter/xt_hashlimit.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 10063408141d..df75ad643eef 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -463,21 +463,19 @@ static u32 xt_hashlimit_len_to_chunks(u32 len)
/* Precision saver. */
static u64 user2credits(u64 user, int revision)
{
+ /* Avoid overflow: divide the constant operands first */
if (revision == 1) {
- /* If multiplying would overflow... */
- if (user > 0x / (HZ*CREDITS_PER_JIFFY_v1))
- /* Divide first. */
- return div64_u64(user, XT_HASHLIMIT_SCALE)
- * HZ * CREDITS_PER_JIFFY_v1;
+ return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE,
+ HZ * CREDITS_PER_JIFFY_v1));
- return div64_u64(user * HZ * CREDITS_PER_JIFFY_v1,
+ return user * div64_u64(HZ * CREDITS_PER_JIFFY_v1,
XT_HASHLIMIT_SCALE);
} else {
- if (user > 0xULL / (HZ*CREDITS_PER_JIFFY))
- return div64_u64(user, XT_HASHLIMIT_SCALE_v2)
- * HZ * CREDITS_PER_JIFFY;
+ if (XT_HASHLIMIT_SCALE_v2 >= HZ * CREDITS_PER_JIFFY)
+ return div64_u64(user, div64_u64(XT_HASHLIMIT_SCALE_v2,
+ HZ * CREDITS_PER_JIFFY));
- return div64_u64(user * HZ * CREDITS_PER_JIFFY,
+ return user * div64_u64(HZ * CREDITS_PER_JIFFY,
XT_HASHLIMIT_SCALE_v2);
}
}
--
2.11.0
Re: Unable to boot linux-next build
Le dimanche 11 septembre 2016 à 20:00 +0200, Quentin Lambert a écrit :
>
> On 09/09/2016 10:31, Quentin Lambert wrote:
> > I have been trying to build and boot the last version available
> > on linux-next.
> > During the build I am being prompted with "has no CRC!" warnings
> > for a bunch
> > of modules.
There is a thread about this issue, with patches on the list (lkml)
titled "[GIT PULL] kbuild changes for v4.9-rc1" starting on Mon, 17 Oct
2016, with initial author "Michal Marek".
To sum up, you could disable CONFIG_MODVERSIONS in your config or
downgrade binutils to <= 2.26 or revert 784d5699eddc ("x86: move
exports to actual definitions") or apply the two patches from the
above thread if on x86:
- first one : from Nicholas Piggin for all archs:
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250340.htm
l
- second from Adam Borowski , x86 specific bits :
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250599.htm
l
Re: Unable to boot linux-next build
Le dimanche 11 septembre 2016 à 20:00 +0200, Quentin Lambert a écrit :
>
> On 09/09/2016 10:31, Quentin Lambert wrote:
> > I have been trying to build and boot the last version available
> > on linux-next.
> > During the build I am being prompted with "has no CRC!" warnings
> > for a bunch
> > of modules.
There is a thread about this issue, with patches on the list (lkml)
titled "[GIT PULL] kbuild changes for v4.9-rc1" starting on Mon, 17 Oct
2016, with initial author "Michal Marek".
To sum up, you could disable CONFIG_MODVERSIONS in your config or
downgrade binutils to <= 2.26 or revert 784d5699eddc ("x86: move
exports to actual definitions") or apply the two patches from the
above thread if on x86:
- first one : from Nicholas Piggin for all archs:
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250340.htm
l
- second from Adam Borowski , x86 specific bits :
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1250599.htm
l
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le vendredi 16 septembre 2016 à 13:55 +0100, Marc Zyngier a écrit : > At that stage, I'm not sure I should care. Does the workaround make > your > platform usable? However awkward this was all about logs and clearing worries. There was no change in behavior between the three cases : upstream, upstream commit reverted and patched upstream. Mind this hardware (the Odroid U2) has no hardware buttons. Only power on plug. Best regards, Alban
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le vendredi 16 septembre 2016 à 13:55 +0100, Marc Zyngier a écrit : > At that stage, I'm not sure I should care. Does the workaround make > your > platform usable? However awkward this was all about logs and clearing worries. There was no change in behavior between the three cases : upstream, upstream commit reverted and patched upstream. Mind this hardware (the Odroid U2) has no hardware buttons. Only power on plug. Best regards, Alban
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit : > Hi Alban, > > On 16/09/16 00:02, Alban Browaeys wrote: > > I am seeing this on arm odroid u2 devicetree : > > genirq: Setting trigger mode 0 for irq 16 failed > > (gic_set_type+0x0/0x64) > > Passing IRQ_TYPE_NONE to a cascading interrupt is risky at best... > Can you point me to the various DTs and their failing interrupts? mine is: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts I got a report of this issue to another odroid : https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts they both get their settings from : https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi relevant in the chain are: - combiner modified: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460 - gic: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576 - gic and combiner initial settings: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134 > Also, can you please give the following patch a go and let me know > if that fixes the issue (I'm interested in the potential warning > here). 1st batch of warnings is : [ cut here ] WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30 Hardware name: ODROID-U2/U3 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xa8/0xd4) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (__irq_do_set_handler+0x1c0/0x1c4) [] (__irq_do_set_handler) from [] (irq_set_chained_handler_and_data+0x38/0x54) [] (irq_set_chained_handler_and_data) from [] (combiner_of_init+0x1a0/0x1c4) [] (combiner_of_init) from [] (of_irq_init+0x194/0x2e8) [] (of_irq_init) from [] (exynos_init_irq+0x8/0x3c) [] (exynos_init_irq) from [] (init_IRQ+0x2c/0x88) [] (init_IRQ) from [] (start_kernel+0x284/0x388) [] (start_kernel) from [<40008078>] (0x40008078) ---[ end trace f68728a0d3053b52 ]--- 2nd batch is : [ cut here ] WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4 Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW 4.8.0-rc6-debug+ #30 Hardware name: ODROID-U2/U3 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xa8/0xd4) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (__irq_do_set_handler+0x1c0/0x1c4) [] (__irq_do_set_handler) from [] (irq_set_chained_handler_and_data+0x38/0x54) [] (irq_set_chained_handler_and_data) from [] (exynos_eint_wkup_init+0x188/0x2dc) [] (exynos_eint_wkup_init) from [] (samsung_pinctrl_probe+0x874/0xa18) [] (samsung_pinctrl_probe) from [] (platform_drv_probe+0x4c/0xb0) [] (platform_drv_probe) from [] (driver_probe_device+0x24c/0x440) [] (driver_probe_device) from [] (bus_for_each_drv+0x64/0x98) [] (bus_for_each_drv) from [] (__device_attach+0xb4/0x144) [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [] (bus_probe_device) from [] (device_add+0x428/0x5c8) [] (device_add) from [] (of_platform_device_create_pdata+0x84/0xb8) [] (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x164/0x440) [] (of_platform_bus_create) from [] (of_platform_populate+0x80/0x114) [] (of_platform_populate) from [] (of_platform_default_populate_init+0x6c/0x80) [] (of_platform_default_populate_init) from [] (do_one_initcall+0x50/0x198) [] (do_one_initcall) from [] (kernel_init_freeable+0x250/0x2f0) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) [] (kernel_init) from [] (ret_from_fork+0x14/0x24) ---[ end trace f68728a0d3053b66 ]--- Best regards, Alban
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le vendredi 16 septembre 2016 à 08:51 +0100, Marc Zyngier a écrit : > Hi Alban, > > On 16/09/16 00:02, Alban Browaeys wrote: > > I am seeing this on arm odroid u2 devicetree : > > genirq: Setting trigger mode 0 for irq 16 failed > > (gic_set_type+0x0/0x64) > > Passing IRQ_TYPE_NONE to a cascading interrupt is risky at best... > Can you point me to the various DTs and their failing interrupts? mine is: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidu3.dts I got a report of this issue to another odroid : https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412-odroidx2.dts they both get their settings from : https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4412.dtsi relevant in the chain are: - combiner modified: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12.dtsi#n460 - gic: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4x12-pinctrl.dtsi#n576 - gic and combiner initial settings: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/exynos4.dtsi#n134 > Also, can you please give the following patch a go and let me know > if that fixes the issue (I'm interested in the potential warning > here). 1st batch of warnings is : [ cut here ] WARNING: CPU: 0 PID: 0 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4 Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc6-debug+ #30 Hardware name: ODROID-U2/U3 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xa8/0xd4) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (__irq_do_set_handler+0x1c0/0x1c4) [] (__irq_do_set_handler) from [] (irq_set_chained_handler_and_data+0x38/0x54) [] (irq_set_chained_handler_and_data) from [] (combiner_of_init+0x1a0/0x1c4) [] (combiner_of_init) from [] (of_irq_init+0x194/0x2e8) [] (of_irq_init) from [] (exynos_init_irq+0x8/0x3c) [] (exynos_init_irq) from [] (init_IRQ+0x2c/0x88) [] (init_IRQ) from [] (start_kernel+0x284/0x388) [] (start_kernel) from [<40008078>] (0x40008078) ---[ end trace f68728a0d3053b52 ]--- 2nd batch is : [ cut here ] WARNING: CPU: 1 PID: 1 at kernel/irq/chip.c:833 __irq_do_set_handler+0x1c0/0x1c4 Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Tainted: GW 4.8.0-rc6-debug+ #30 Hardware name: ODROID-U2/U3 [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xa8/0xd4) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (__irq_do_set_handler+0x1c0/0x1c4) [] (__irq_do_set_handler) from [] (irq_set_chained_handler_and_data+0x38/0x54) [] (irq_set_chained_handler_and_data) from [] (exynos_eint_wkup_init+0x188/0x2dc) [] (exynos_eint_wkup_init) from [] (samsung_pinctrl_probe+0x874/0xa18) [] (samsung_pinctrl_probe) from [] (platform_drv_probe+0x4c/0xb0) [] (platform_drv_probe) from [] (driver_probe_device+0x24c/0x440) [] (driver_probe_device) from [] (bus_for_each_drv+0x64/0x98) [] (bus_for_each_drv) from [] (__device_attach+0xb4/0x144) [] (__device_attach) from [] (bus_probe_device+0x88/0x90) [] (bus_probe_device) from [] (device_add+0x428/0x5c8) [] (device_add) from [] (of_platform_device_create_pdata+0x84/0xb8) [] (of_platform_device_create_pdata) from [] (of_platform_bus_create+0x164/0x440) [] (of_platform_bus_create) from [] (of_platform_populate+0x80/0x114) [] (of_platform_populate) from [] (of_platform_default_populate_init+0x6c/0x80) [] (of_platform_default_populate_init) from [] (do_one_initcall+0x50/0x198) [] (do_one_initcall) from [] (kernel_init_freeable+0x250/0x2f0) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) [] (kernel_init) from [] (ret_from_fork+0x14/0x24) ---[ end trace f68728a0d3053b66 ]--- Best regards, Alban
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le mercredi 14 septembre 2016 à 21:25 +0200, Geert Uytterhoeven a
écrit :
> JFYI, with v4.8-rc6 I'm seeing
>
> genirq: Setting trigger mode 0 for irq 11 failed
> (txx9_irq_set_type+0x0/0xb8)
>
> on rbtx4927. This did not happen with v4.8-rc3.
txx9_irq_set_type receives a type IRQ_TYPE_NONE from the call to
__irq_set_trigger added in:
1e12c4a939 ("genirq: Correctly configure the trigger on chained interrupts")
This patch is a regression fix for :
Desc: irqdomain: Don't set type when mapping an IRQ breaks nexus7 gpio buttons
Repo: 2016-07-30 https://marc.info/?l=linux-kernel=146985356305280=2
I am seeing this on arm odroid u2 devicetree :
genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x64)
Alban
Re: genirq: Setting trigger mode 0 for irq 11 failed (txx9_irq_set_type+0x0/0xb8)
Le mercredi 14 septembre 2016 à 21:25 +0200, Geert Uytterhoeven a
écrit :
> JFYI, with v4.8-rc6 I'm seeing
>
> genirq: Setting trigger mode 0 for irq 11 failed
> (txx9_irq_set_type+0x0/0xb8)
>
> on rbtx4927. This did not happen with v4.8-rc3.
txx9_irq_set_type receives a type IRQ_TYPE_NONE from the call to
__irq_set_trigger added in:
1e12c4a939 ("genirq: Correctly configure the trigger on chained interrupts")
This patch is a regression fix for :
Desc: irqdomain: Don't set type when mapping an IRQ breaks nexus7 gpio buttons
Repo: 2016-07-30 https://marc.info/?l=linux-kernel=146985356305280=2
I am seeing this on arm odroid u2 devicetree :
genirq: Setting trigger mode 0 for irq 16 failed (gic_set_type+0x0/0x64)
Alban
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Le jeudi 05 février 2015 à 13:45 -0800, Stephen Boyd a écrit : > On 02/05/15 13:30, Alban Browaeys wrote: > >> Thanks for the information. Can you please try the patch in this other > >> thread[1]? I think you're seeing the same problem. > >> > >> [1] https://lkml.org/lkml/2015/2/5/595 > > > > This fixes both oops : the one from __clk_get and the next from > > hlist_del. > > > > Great! Can I take this as your Tested-by? Tested-by: Alban Browaeys Well done! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Le jeudi 05 février 2015 à 13:45 -0800, Stephen Boyd a écrit : On 02/05/15 13:30, Alban Browaeys wrote: Thanks for the information. Can you please try the patch in this other thread[1]? I think you're seeing the same problem. [1] https://lkml.org/lkml/2015/2/5/595 This fixes both oops : the one from __clk_get and the next from hlist_del. Great! Can I take this as your Tested-by? Tested-by: Alban Browaeys pra...@yahoo.com Well done! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
> > Thanks for the information. Can you please try the patch in this other > thread[1]? I think you're seeing the same problem. > > [1] https://lkml.org/lkml/2015/2/5/595 This fixes both oops : the one from __clk_get and the next from hlist_del. Thanks Alban -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.
Le jeudi 05 février 2015 à 11:33 -0800, Stephen Boyd a écrit : > On 02/05/15 10:24, Alban Browaeys wrote: > > Put is called more than once, thus hlist_del ends up on a poisoned > > list. > > Why is clk_put() called more than once on the same clk pointer? Where > does this happe > With only patch 1 from this set applied I get another oops: [7.346612] Unable to handle kernel paging request at virtual address 00200200 [7.353686] pgd = ebbf4000 [7.355939] [00200200] *pgd= [7.359444] Internal error: Oops: 805 [#1] PREEMPT SMP ARM [7.364914] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 fuse [7.372201] CPU: 3 PID: 102 Comm: systemd-modules Not tainted 3.19.0-rc7-next-20150204-00053-g17e4521 #93 [7.381764] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [7.387832] task: eb0a8f00 ti: ebbfe000 task.ti: ebbfe000 [7.393213] PC is at __clk_put+0x64/0xdc [7.397111] LR is at clk_prepare_lock+0x20/0x100 [7.401712] pc : []lr : []psr: 200f0053 [7.401712] sp : ebbffbb8 ip : ebbffba0 fp : ebbffbcc [7.413185] r10: ee0ff600 r9 : 0001 r8 : [7.413188] r7 : ebbffbf8 r6 : r5 : ee5c7d80 r4 : ee0ff600 [7.413195] r3 : 00100100 r2 : 00200200 r1 : 080007ff r0 : 0001 [7.413200] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment user [7.413204] Control: 10c5387d Table: 6bbf404a DAC: 0015 [7.413208] Process systemd-modules (pid: 102, stack limit = 0xebbfe218) [7.413211] Stack: (0xebbffbb8 to 0xebc0) [7.413215] fba0: ee106400 [7.413222] fbc0: ebbffbdc ebbffbd0 c055222c c0556114 ebbffc6c ebbffbe0 c0558604 c0552220 [7.413228] fbe0: ebbffbf8 c01cc560 ebbbc310 ee2b5200 ebbffc14 c01ced00 ee5e0d3c 0001 [7.413234] fc00: 0181 ee2b5200 ebbffc34 ebbbc100 c08c5790 ee2b5200 ebbbc300 [7.413239] fc20: 0001 0006 ebbffc5c ebbffc38 c01ced00 c01cb2d0 ed834850 [7.413245] fc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8 bf06c0b4 0006 [7.413251] fc60: ebbffc8c ebbffc70 c044213c c055846c ed834850 c0b61248 c0b61254 c0aa82b8 [7.413257] fc80: ebbffcc4 ebbffc90 c043ff34 c0442120 ed834850 bf06c0b4 ed834884 ed834850 [7.413263] fca0: bf06c0b4 ed834884 bf0631f0 eb3cfc00 c0a4f40c ebbffce4 ebbffcc8 [7.413269] fcc0: c044020c c043fdc8 bf06c0b4 c0440194 ebbffd0c ebbffce8 [7.413275] fce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5720 bf06c0b4 eb329cc0 c0a87448 [7.413280] fd00: ebbffd1c ebbffd10 c043fa14 c043de88 ebbffd44 ebbffd20 c043f460 c043f9f4 [7.413286] fd20: bf069280 ebbffd30 bf06c0b4 bf0631e8 bf06c388 ebbffd5c ebbffd48 [7.413292] fd40: c0440bfc c043f370 0001 ebbffd6c ebbffd60 c0442094 c0440b50 [7.413298] fd60: ebbffdbc ebbffd70 bf04ca08 c044203c bf065090 [7.413303] fd80: c0a53b20 bf06c208 [7.413309] fda0: c0a53b20 bf04c950 c0a53b20 ebbffe4c ebbffdc0 c0008b28 bf04c95c [7.413315] fdc0: 001f ebbffdec ebbffdd8 c00504f4 c006dde0 ebbfe000 [7.413321] fde0: ee002140 00d0 c06ed168 000c c0a50600 0001 ebbffe4c ebbffe08 [7.413327] fe00: c01541d8 c0153934 ebbfe008 ebbffe08 0001 ebbfe008 ee002140 dc8cb100 [7.41] fe20: 0001 bf06c208 0001 eb3cfd00 eb3cf000 0001 14c3101c eb3cf008 [7.413339] fe40: ebbffe74 ebbffe50 c06ed1a4 c00089ec ebbffe74 ebbffe60 c014496c ebbfff48 [7.413345] fe60: 0001 bf06c208 ebbfff3c ebbffe78 c00af61c c06ed140 bf06c214 7fff [7.413350] fe80: c00ac6a8 ebbfff48 ebbffeb4 f0473db8 0780 0777 f0473e84 bf06c214 [7.413356] fea0: bf06c378 b6dc99f8 bf06c250 c0a4f40c c00ad024 c0169924 [7.413362] fec0: bf063194 0009 6e72656b 6c65 [7.413367] fee0: [7.413373] ff00: dc8cb100 ebbfff2c 0006 b6dc99f8 [7.413379] ff20: 017b c000fb64 ebbfe000 ebbfffa4 ebbfff40 c00afdec c00ada4c [7.413385] ff40: c0180738 f0454000 002bb414 f070ec6c f066643f f066dd80 00020390 [7.413390] ff60: 00026450 bf06c1f0 0001 002f 0030 001a [7.413396] ff80: 0008 b6dca7d4 00028948 e1c5e100 ebbfffa8 [7.413402] ffa0: c000f9c0 c00afd54 b6dca7d4 00028948 0006 b6dc99f8 b6dca31c [7.413408] ffc0: b6dca7d4 00028948 e1c5e100 017b 0002 00015964 00015f34 0002e640 [7.413414] ffe0: bedc0268 bedc0258 b6dc3c4b b6e6cd42 600d0070 0006 6f7fd821 6f7fdc21 [7.413427] [] (__clk_put) from [] (clk_put+0x18/0x1c) [7.413438] [] (clk_put) from [] (of_c
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Le jeudi 05 février 2015 à 11:30 -0800, Stephen Boyd a écrit :
> > Signed-off-by: Alban Browaeys
> > ---
> > drivers/clk/clk.c | 17 +
> > 1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index df94668..8f33722 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
> > */
> > static void __clk_release(struct kref *ref)
> > {
> > - struct clk_core *clk = container_of(ref, struct clk_core, ref);
> > - int i = clk->num_parents;
> > + struct clk_core *core = container_of(ref, struct clk_core, ref);
> > + struct clk *clk = container_of(, struct clk, core);
>
> How does this work? struct clk_core doesn't have a struct clk inside it.
>
Seems I am confused. The aim is to get the clk struct from its core
field. If I cannot do that from within __clk_release , this fix is
doomed.
> > + int i = core->num_parents;
> >
> > - kfree(clk->parents);
> > + kfree(core->parents);
> > while (--i >= 0)
> > - kfree_const(clk->parent_names[i]);
> > + kfree_const(core->parent_names[i]);
>
> We don't have kfree_const() in the clk-next tree so please resend based
> on clk-next, not linux-next.
>
I will do after we confirmed there is a way to get to clk from clk_core.
Otherwise the fix makes no sense.
> I'm still confused. Care to send the actual backtrace and describe which
> hardware you're running on (perhaps some dts file to look at)?
>
This is the initial oops before any change (based on linux-next
20150204).
[7.264186] Unable to handle kernel paging request at virtual address
6b6b6b77
[7.270206] pgd = eb0b4000
[7.272809] [6b6b6b77] *pgd=
[7.276466] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[7.281667] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2
fuse
[7.288950] CPU: 1 PID: 98 Comm: systemd-modules Not tainted
3.19.0-rc7-next-20150204-00052-g1059e6a #91
[7.298424] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[7.304488] task: ebae3c00 ti: eb0bc000 task.ti: eb0bc000
[7.309888] PC is at __clk_get+0x30/0xa0
[7.313781] LR is at of_clk_get_by_clkspec+0x38/0x54
[7.318722] pc : []lr : []psr: 200d0053
[7.318722] sp : eb0bdbb0 ip : eb0bdbc8 fp : eb0bdbc4
[7.330187] r10: 0006 r9 : 0001 r8 :
[7.335398] r7 : eb0bdbf8 r6 : r5 : ee5c7d80 r4 : 6b6b6b6b
[7.341913] r3 : 0001 r2 : 0011 r1 : ee0b7004 r0 : ee0ff600
[7.341923] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment
user
[7.341927] Control: 10c5387d Table: 6b0b404a DAC: 0015
[7.341931] Process systemd-modules (pid: 98, stack limit = 0xeb0bc218)
[7.341934] Stack: (0xeb0bdbb0 to 0xeb0be000)
[7.341939] dba0: 0001 ee0ff600
eb0bdbdc eb0bdbc8
[7.341945] dbc0: c055231c c0556074 0001 ed834850 eb0bdc6c eb0bdbe0
c0558528 c05522f0
[7.341950] dbe0: eb0bdbf8 c01cc560 eb3e4710 ee2b4200 eb0bdc14 c01ced00
ee5e0d3c 0001
[7.341956] dc00: 0011 ee2b4200 eb0bdc34 eb3e4900 c08c5790
ee2b4200 eb3e4700
[7.341962] dc20: 0001 0006 eb0bdc5c eb0bdc38 c01ced00 c01cb2d0
ed834850
[7.341968] dc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8
bf06c0b4 0006
[7.341974] dc60: eb0bdc8c eb0bdc70 c044213c c0558474 ed834850 c0b61248
c0b61254 c0aa82b8
[7.341979] dc80: eb0bdcc4 eb0bdc90 c043ff34 c0442120 bf0631f0
e9c3b700 ed834850
[7.341985] dca0: bf06c0b4 ed834884 bf0631f0 e9c3b700 c0a4f40c
eb0bdce4 eb0bdcc8
[7.341991] dcc0: c044020c c043fdc8 bf06c0b4 c0440194
eb0bdd0c eb0bdce8
[7.341997] dce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5728 bf06c0b4
eb1477c0 c0a87448
[7.342003] dd00: eb0bdd1c eb0bdd10 c043fa14 c043de88 eb0bdd44 eb0bdd20
c043f460 c043f9f4
[7.342009] dd20: bf069280 eb0bdd30 bf06c0b4 bf0631e8 bf06c388
eb0bdd5c eb0bdd48
[7.342014] dd40: c0440bfc c043f370 0001 eb0bdd6c eb0bdd60
c0442094 c0440b50
[7.342020] dd60: eb0bddbc eb0bdd70 bf04ca08 c044203c bf065090
[7.342026] dd80:
c0a53b20 bf06c208
[7.342031] dda0: c0a53b20 bf04c950 c0a53b20 eb0bde4c eb0bddc0
c0008b28 bf04c95c
[7.342037] ddc0: 001f eb0bddec eb0bddd8 c00504f4 c006dde0
eb0bc000
[7.342043] dde0: ee002140 00d0 c06ed170 000c c0a50600
eb0bde4c eb0bde08
[7.342049] de00: c01541d8 c0153934 eb0bc008 eb0bde08 eb0bc008
ee002140 dc8cb100
[7.342055] de20: 0001 bf06c208 0001 e9c3b600 e9c3bb00 0001
1
[PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.
Put is called more than once, thus hlist_del ends up on a poisoned
list.
Move hlist_del to the __clk_release handler managed by kref instead
of calling it in _clk_put.
Fixes: 1c8e600440c7 ("clk: Add rate constraints to clocks")
Signed-off-by: Alban Browaeys
---
drivers/clk/clk.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8f33722..f1d4b33 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref)
struct clk *clk = container_of(, struct clk, core);
int i = core->num_parents;
+ hlist_del(>child_node);
+ clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
+
kfree(core->parents);
while (--i >= 0)
kfree_const(core->parent_names[i]);
@@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk)
clk_prepare_lock();
- hlist_del(>child_node);
- clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
owner = clk->core->owner;
kref_put(>core->ref, __clk_release);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
On the second call to __set_clk_parents from of_clk_set_defaults, here
when registering the second fimc device the kernel OOPS in an "unhandled
paging request at virtual address 6b6b6b77". This in __clk_get when
dereferencing clk->owner.
Move the clk free in the kref managed _clk_release call instead of
plain __clk_put.
Fixes: 035a61c314eb ("clk: Make clk API return per-user struct clk
instances)
Signed-off-by: Alban Browaeys
---
drivers/clk/clk.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index df94668..8f33722 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
*/
static void __clk_release(struct kref *ref)
{
- struct clk_core *clk = container_of(ref, struct clk_core, ref);
- int i = clk->num_parents;
+ struct clk_core *core = container_of(ref, struct clk_core, ref);
+ struct clk *clk = container_of(, struct clk, core);
+ int i = core->num_parents;
- kfree(clk->parents);
+ kfree(core->parents);
while (--i >= 0)
- kfree_const(clk->parent_names[i]);
+ kfree_const(core->parent_names[i]);
+
+ kfree(core->parent_names);
+ kfree_const(core->name);
+ kfree(core);
- kfree(clk->parent_names);
- kfree_const(clk->name);
kfree(clk);
}
@@ -2671,8 +2674,6 @@ void __clk_put(struct clk *clk)
clk_prepare_unlock();
module_put(owner);
-
- kfree(clk);
}
/***clk rate change notifiers***/
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.
Le jeudi 05 février 2015 à 11:33 -0800, Stephen Boyd a écrit : On 02/05/15 10:24, Alban Browaeys wrote: Put is called more than once, thus hlist_del ends up on a poisoned list. Why is clk_put() called more than once on the same clk pointer? Where does this happe With only patch 1 from this set applied I get another oops: [7.346612] Unable to handle kernel paging request at virtual address 00200200 [7.353686] pgd = ebbf4000 [7.355939] [00200200] *pgd= [7.359444] Internal error: Oops: 805 [#1] PREEMPT SMP ARM [7.364914] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2 fuse [7.372201] CPU: 3 PID: 102 Comm: systemd-modules Not tainted 3.19.0-rc7-next-20150204-00053-g17e4521 #93 [7.381764] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [7.387832] task: eb0a8f00 ti: ebbfe000 task.ti: ebbfe000 [7.393213] PC is at __clk_put+0x64/0xdc [7.397111] LR is at clk_prepare_lock+0x20/0x100 [7.401712] pc : [c055616c]lr : [c0552bfc]psr: 200f0053 [7.401712] sp : ebbffbb8 ip : ebbffba0 fp : ebbffbcc [7.413185] r10: ee0ff600 r9 : 0001 r8 : [7.413188] r7 : ebbffbf8 r6 : r5 : ee5c7d80 r4 : ee0ff600 [7.413195] r3 : 00100100 r2 : 00200200 r1 : 080007ff r0 : 0001 [7.413200] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment user [7.413204] Control: 10c5387d Table: 6bbf404a DAC: 0015 [7.413208] Process systemd-modules (pid: 102, stack limit = 0xebbfe218) [7.413211] Stack: (0xebbffbb8 to 0xebc0) [7.413215] fba0: ee106400 [7.413222] fbc0: ebbffbdc ebbffbd0 c055222c c0556114 ebbffc6c ebbffbe0 c0558604 c0552220 [7.413228] fbe0: ebbffbf8 c01cc560 ebbbc310 ee2b5200 ebbffc14 c01ced00 ee5e0d3c 0001 [7.413234] fc00: 0181 ee2b5200 ebbffc34 ebbbc100 c08c5790 ee2b5200 ebbbc300 [7.413239] fc20: 0001 0006 ebbffc5c ebbffc38 c01ced00 c01cb2d0 ed834850 [7.413245] fc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8 bf06c0b4 0006 [7.413251] fc60: ebbffc8c ebbffc70 c044213c c055846c ed834850 c0b61248 c0b61254 c0aa82b8 [7.413257] fc80: ebbffcc4 ebbffc90 c043ff34 c0442120 ed834850 bf06c0b4 ed834884 ed834850 [7.413263] fca0: bf06c0b4 ed834884 bf0631f0 eb3cfc00 c0a4f40c ebbffce4 ebbffcc8 [7.413269] fcc0: c044020c c043fdc8 bf06c0b4 c0440194 ebbffd0c ebbffce8 [7.413275] fce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5720 bf06c0b4 eb329cc0 c0a87448 [7.413280] fd00: ebbffd1c ebbffd10 c043fa14 c043de88 ebbffd44 ebbffd20 c043f460 c043f9f4 [7.413286] fd20: bf069280 ebbffd30 bf06c0b4 bf0631e8 bf06c388 ebbffd5c ebbffd48 [7.413292] fd40: c0440bfc c043f370 0001 ebbffd6c ebbffd60 c0442094 c0440b50 [7.413298] fd60: ebbffdbc ebbffd70 bf04ca08 c044203c bf065090 [7.413303] fd80: c0a53b20 bf06c208 [7.413309] fda0: c0a53b20 bf04c950 c0a53b20 ebbffe4c ebbffdc0 c0008b28 bf04c95c [7.413315] fdc0: 001f ebbffdec ebbffdd8 c00504f4 c006dde0 ebbfe000 [7.413321] fde0: ee002140 00d0 c06ed168 000c c0a50600 0001 ebbffe4c ebbffe08 [7.413327] fe00: c01541d8 c0153934 ebbfe008 ebbffe08 0001 ebbfe008 ee002140 dc8cb100 [7.41] fe20: 0001 bf06c208 0001 eb3cfd00 eb3cf000 0001 14c3101c eb3cf008 [7.413339] fe40: ebbffe74 ebbffe50 c06ed1a4 c00089ec ebbffe74 ebbffe60 c014496c ebbfff48 [7.413345] fe60: 0001 bf06c208 ebbfff3c ebbffe78 c00af61c c06ed140 bf06c214 7fff [7.413350] fe80: c00ac6a8 ebbfff48 ebbffeb4 f0473db8 0780 0777 f0473e84 bf06c214 [7.413356] fea0: bf06c378 b6dc99f8 bf06c250 c0a4f40c c00ad024 c0169924 [7.413362] fec0: bf063194 0009 6e72656b 6c65 [7.413367] fee0: [7.413373] ff00: dc8cb100 ebbfff2c 0006 b6dc99f8 [7.413379] ff20: 017b c000fb64 ebbfe000 ebbfffa4 ebbfff40 c00afdec c00ada4c [7.413385] ff40: c0180738 f0454000 002bb414 f070ec6c f066643f f066dd80 00020390 [7.413390] ff60: 00026450 bf06c1f0 0001 002f 0030 001a [7.413396] ff80: 0008 b6dca7d4 00028948 e1c5e100 ebbfffa8 [7.413402] ffa0: c000f9c0 c00afd54 b6dca7d4 00028948 0006 b6dc99f8 b6dca31c [7.413408] ffc0: b6dca7d4 00028948 e1c5e100 017b 0002 00015964 00015f34 0002e640 [7.413414] ffe0: bedc0268 bedc0258 b6dc3c4b b6e6cd42 600d0070 0006 6f7fd821 6f7fdc21 [7.413427] [c055616c] (__clk_put) from [c055222c] (clk_put+0x18/0x1c) [7.413438] [c055222c] (clk_put) from [c0558604
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Thanks for the information. Can you please try the patch in this other thread[1]? I think you're seeing the same problem. [1] https://lkml.org/lkml/2015/2/5/595 This fixes both oops : the one from __clk_get and the next from hlist_del. Thanks Alban -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] clk: Fix OOPS calling hlist_del on an already poisoned hlist.
Put is called more than once, thus hlist_del ends up on a poisoned list. Move hlist_del to the __clk_release handler managed by kref instead of calling it in _clk_put. Fixes: 1c8e600440c7 (clk: Add rate constraints to clocks) Signed-off-by: Alban Browaeys pra...@yahoo.com --- drivers/clk/clk.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 8f33722..f1d4b33 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2489,6 +2489,9 @@ static void __clk_release(struct kref *ref) struct clk *clk = container_of(core, struct clk, core); int i = core-num_parents; + hlist_del(clk-child_node); + clk_core_set_rate_nolock(clk-core, clk-core-req_rate); + kfree(core-parents); while (--i = 0) kfree_const(core-parent_names[i]); @@ -2666,8 +2669,6 @@ void __clk_put(struct clk *clk) clk_prepare_lock(); - hlist_del(clk-child_node); - clk_core_set_rate_nolock(clk-core, clk-core-req_rate); owner = clk-core-owner; kref_put(clk-core-ref, __clk_release); -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
On the second call to __set_clk_parents from of_clk_set_defaults, here
when registering the second fimc device the kernel OOPS in an unhandled
paging request at virtual address 6b6b6b77. This in __clk_get when
dereferencing clk-owner.
Move the clk free in the kref managed _clk_release call instead of
plain __clk_put.
Fixes: 035a61c314eb (clk: Make clk API return per-user struct clk
instances)
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
drivers/clk/clk.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index df94668..8f33722 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
*/
static void __clk_release(struct kref *ref)
{
- struct clk_core *clk = container_of(ref, struct clk_core, ref);
- int i = clk-num_parents;
+ struct clk_core *core = container_of(ref, struct clk_core, ref);
+ struct clk *clk = container_of(core, struct clk, core);
+ int i = core-num_parents;
- kfree(clk-parents);
+ kfree(core-parents);
while (--i = 0)
- kfree_const(clk-parent_names[i]);
+ kfree_const(core-parent_names[i]);
+
+ kfree(core-parent_names);
+ kfree_const(core-name);
+ kfree(core);
- kfree(clk-parent_names);
- kfree_const(clk-name);
kfree(clk);
}
@@ -2671,8 +2674,6 @@ void __clk_put(struct clk *clk)
clk_prepare_unlock();
module_put(owner);
-
- kfree(clk);
}
/***clk rate change notifiers***/
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] clk: Fix __clk_get access to already freed owner field.
Le jeudi 05 février 2015 à 11:30 -0800, Stephen Boyd a écrit :
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
drivers/clk/clk.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index df94668..8f33722 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2485,15 +2485,18 @@ EXPORT_SYMBOL_GPL(clk_register);
*/
static void __clk_release(struct kref *ref)
{
- struct clk_core *clk = container_of(ref, struct clk_core, ref);
- int i = clk-num_parents;
+ struct clk_core *core = container_of(ref, struct clk_core, ref);
+ struct clk *clk = container_of(core, struct clk, core);
How does this work? struct clk_core doesn't have a struct clk inside it.
Seems I am confused. The aim is to get the clk struct from its core
field. If I cannot do that from within __clk_release , this fix is
doomed.
+ int i = core-num_parents;
- kfree(clk-parents);
+ kfree(core-parents);
while (--i = 0)
- kfree_const(clk-parent_names[i]);
+ kfree_const(core-parent_names[i]);
We don't have kfree_const() in the clk-next tree so please resend based
on clk-next, not linux-next.
I will do after we confirmed there is a way to get to clk from clk_core.
Otherwise the fix makes no sense.
I'm still confused. Care to send the actual backtrace and describe which
hardware you're running on (perhaps some dts file to look at)?
This is the initial oops before any change (based on linux-next
20150204).
[7.264186] Unable to handle kernel paging request at virtual address
6b6b6b77
[7.270206] pgd = eb0b4000
[7.272809] [6b6b6b77] *pgd=
[7.276466] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[7.281667] Modules linked in: exynosdrm(+) drm_kms_helper phy_exynos_usb2
fuse
[7.288950] CPU: 1 PID: 98 Comm: systemd-modules Not tainted
3.19.0-rc7-next-20150204-00052-g1059e6a #91
[7.298424] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[7.304488] task: ebae3c00 ti: eb0bc000 task.ti: eb0bc000
[7.309888] PC is at __clk_get+0x30/0xa0
[7.313781] LR is at of_clk_get_by_clkspec+0x38/0x54
[7.318722] pc : [c0556098]lr : [c055231c]psr: 200d0053
[7.318722] sp : eb0bdbb0 ip : eb0bdbc8 fp : eb0bdbc4
[7.330187] r10: 0006 r9 : 0001 r8 :
[7.335398] r7 : eb0bdbf8 r6 : r5 : ee5c7d80 r4 : 6b6b6b6b
[7.341913] r3 : 0001 r2 : 0011 r1 : ee0b7004 r0 : ee0ff600
[7.341923] Flags: nzCv IRQs on FIQs off Mode SVC_32 ISA ARM Segment
user
[7.341927] Control: 10c5387d Table: 6b0b404a DAC: 0015
[7.341931] Process systemd-modules (pid: 98, stack limit = 0xeb0bc218)
[7.341934] Stack: (0xeb0bdbb0 to 0xeb0be000)
[7.341939] dba0: 0001 ee0ff600
eb0bdbdc eb0bdbc8
[7.341945] dbc0: c055231c c0556074 0001 ed834850 eb0bdc6c eb0bdbe0
c0558528 c05522f0
[7.341950] dbe0: eb0bdbf8 c01cc560 eb3e4710 ee2b4200 eb0bdc14 c01ced00
ee5e0d3c 0001
[7.341956] dc00: 0011 ee2b4200 eb0bdc34 eb3e4900 c08c5790
ee2b4200 eb3e4700
[7.341962] dc20: 0001 0006 eb0bdc5c eb0bdc38 c01ced00 c01cb2d0
ed834850
[7.341968] dc40: ed834858 ed834850 ed834850 bf06c0b4 c0aa82b8
bf06c0b4 0006
[7.341974] dc60: eb0bdc8c eb0bdc70 c044213c c0558474 ed834850 c0b61248
c0b61254 c0aa82b8
[7.341979] dc80: eb0bdcc4 eb0bdc90 c043ff34 c0442120 bf0631f0
e9c3b700 ed834850
[7.341985] dca0: bf06c0b4 ed834884 bf0631f0 e9c3b700 c0a4f40c
eb0bdce4 eb0bdcc8
[7.341991] dcc0: c044020c c043fdc8 bf06c0b4 c0440194
eb0bdd0c eb0bdce8
[7.341997] dce0: c043ded8 c04401a0 ee284e38 ed830900 c06f5728 bf06c0b4
eb1477c0 c0a87448
[7.342003] dd00: eb0bdd1c eb0bdd10 c043fa14 c043de88 eb0bdd44 eb0bdd20
c043f460 c043f9f4
[7.342009] dd20: bf069280 eb0bdd30 bf06c0b4 bf0631e8 bf06c388
eb0bdd5c eb0bdd48
[7.342014] dd40: c0440bfc c043f370 0001 eb0bdd6c eb0bdd60
c0442094 c0440b50
[7.342020] dd60: eb0bddbc eb0bdd70 bf04ca08 c044203c bf065090
[7.342026] dd80:
c0a53b20 bf06c208
[7.342031] dda0: c0a53b20 bf04c950 c0a53b20 eb0bde4c eb0bddc0
c0008b28 bf04c95c
[7.342037] ddc0: 001f eb0bddec eb0bddd8 c00504f4 c006dde0
eb0bc000
[7.342043] dde0: ee002140 00d0 c06ed170 000c c0a50600
eb0bde4c eb0bde08
[7.342049] de00: c01541d8 c0153934 eb0bc008 eb0bde08 eb0bc008
ee002140 dc8cb100
[7.342055] de20: 0001 bf06c208 0001 e9c3b600 e9c3bb00 0001
163c451c e9c3bb08
[7.342060] de40: eb0bde74 eb0bde50 c06ed1ac c00089ec eb0bde74 eb0bde60
c014496c eb0bdf48
[7.342066] de60: 0001 bf06c208 eb0bdf3c eb0bde78 c00af61c c06ed148
bf06c214 7fff
[7.342072] de80
usb: misc: usb3503: connect and intn mismatch in OF vs variable assignments
hub->gpio_intn = of_get_named_gpio(np, "connect-gpios", 0); and hub->gpio_connect = of_get_named_gpio(np, "intn-gpios", 0); I guess they should be switched , that is connect-gpios bound to hub->gpio_connect and intn-gpios to hub->gpio_int. Sorry not to provides a patch. I thought that this issue might ends up with dts relying on this broken behaviour if not pointed early ... Either way if not fixed soon , I will have completed my branch cleanup (I also have other patches to this file ie drivers/usb/misc/usb3503.c). Best regards Alban -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
usb: misc: usb3503: connect and intn mismatch in OF vs variable assignments
hub-gpio_intn = of_get_named_gpio(np, connect-gpios, 0); and hub-gpio_connect = of_get_named_gpio(np, intn-gpios, 0); I guess they should be switched , that is connect-gpios bound to hub-gpio_connect and intn-gpios to hub-gpio_int. Sorry not to provides a patch. I thought that this issue might ends up with dts relying on this broken behaviour if not pointed early ... Either way if not fixed soon , I will have completed my branch cleanup (I also have other patches to this file ie drivers/usb/misc/usb3503.c). Best regards Alban -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance
Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap and vidioc_try_fmt_vid_cap. Catched by v4l2-compliance. Signed-off-by: Alban Browaeys --- drivers/media/usb/em28xx/em28xx-video.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c index 1a577ed..42930a4 100644 --- a/drivers/media/usb/em28xx/em28xx-video.c +++ b/drivers/media/usb/em28xx/em28xx-video.c @@ -943,6 +943,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, else f->fmt.pix.field = dev->interlaced ? V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP; + f->fmt.pix.priv = 0; + return 0; } @@ -1008,6 +1010,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv, else f->fmt.pix.field = dev->interlaced ? V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP; + f->fmt.pix.priv = 0; return 0; } -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] [media] em28xx: usb power config is in the low byte.
According to the em2860 datasheet, eeprom byte 08H is Chip
Configuration Low Byte and 09H is High Byte.
Usb power configuration is in the Low byte (same as the usb audio
class config).
Signed-off-by: Alban Browaeys
---
drivers/media/usb/em28xx/em28xx-i2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index c4ff973..6ff7415 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -743,13 +743,13 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned
bus,
break;
}
- if (le16_to_cpu(dev_config->chip_conf) & 1 << 3)
+ if (le16_to_cpu(dev_config->chip_conf) >> 4 & 1 << 3)
em28xx_info("\tUSB Remote wakeup capable\n");
- if (le16_to_cpu(dev_config->chip_conf) & 1 << 2)
+ if (le16_to_cpu(dev_config->chip_conf) >> 4 & 1 << 2)
em28xx_info("\tUSB Self power capable\n");
- switch (le16_to_cpu(dev_config->chip_conf) & 0x3) {
+ switch (le16_to_cpu(dev_config->chip_conf) >> 4 & 0x3) {
case 0:
em28xx_info("\t500mA max power\n");
break;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] [media] em28xx: i2s 5 sample rates is a subset of 3 one.
As:
EM28XX_CHIPCFG_I2S_3_SAMPRATES 0x20
EM28XX_CHIPCFG_I2S_5_SAMPRATES 0x30
the board chipcfg is 0xf0 thus if 3_SAMPRATES is tested
first and matches while it is a 5_SAMPRATES.
Signed-off-by: Alban Browaeys
---
drivers/media/usb/em28xx/em28xx-core.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index fc157af..3c0c5e9 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -505,13 +505,13 @@ int em28xx_audio_setup(struct em28xx *dev)
dev->audio_mode.has_audio = false;
return 0;
} else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
- EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
- em28xx_info("I2S Audio (3 sample rates)\n");
- dev->audio_mode.i2s_3rates = 1;
- } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
EM28XX_CHIPCFG_I2S_5_SAMPRATES) {
em28xx_info("I2S Audio (5 sample rates)\n");
dev->audio_mode.i2s_5rates = 1;
+ } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) ==
+ EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
+ em28xx_info("I2S Audio (3 sample rates)\n");
+ dev->audio_mode.i2s_3rates = 1;
}
if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] [media] em28xx: fix assignment of the eeprom data.
Set the config structure pointer to the eeprom data pointer (data,
here eedata dereferenced) not the pointer to the pointer to
the eeprom data (eedata itself).
Signed-off-by: Alban Browaeys
---
drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 4851cc2..c4ff973 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -726,7 +726,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned
bus,
*eedata = data;
*eedata_len = len;
- dev_config = (void *)eedata;
+ dev_config = (void *)*eedata;
switch (le16_to_cpu(dev_config->chip_conf) >> 4 & 0x3) {
case 0:
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] [media] em28xx: fix assignment of the eeprom data.
Set the config structure pointer to the eeprom data pointer (data,
here eedata dereferenced) not the pointer to the pointer to
the eeprom data (eedata itself).
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
drivers/media/usb/em28xx/em28xx-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 4851cc2..c4ff973 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -726,7 +726,7 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned
bus,
*eedata = data;
*eedata_len = len;
- dev_config = (void *)eedata;
+ dev_config = (void *)*eedata;
switch (le16_to_cpu(dev_config-chip_conf) 4 0x3) {
case 0:
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/4] [media] em28xx: i2s 5 sample rates is a subset of 3 one.
As:
EM28XX_CHIPCFG_I2S_3_SAMPRATES 0x20
EM28XX_CHIPCFG_I2S_5_SAMPRATES 0x30
the board chipcfg is 0xf0 thus if 3_SAMPRATES is tested
first and matches while it is a 5_SAMPRATES.
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
drivers/media/usb/em28xx/em28xx-core.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index fc157af..3c0c5e9 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -505,13 +505,13 @@ int em28xx_audio_setup(struct em28xx *dev)
dev-audio_mode.has_audio = false;
return 0;
} else if ((cfg EM28XX_CHIPCFG_AUDIOMASK) ==
- EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
- em28xx_info(I2S Audio (3 sample rates)\n);
- dev-audio_mode.i2s_3rates = 1;
- } else if ((cfg EM28XX_CHIPCFG_AUDIOMASK) ==
EM28XX_CHIPCFG_I2S_5_SAMPRATES) {
em28xx_info(I2S Audio (5 sample rates)\n);
dev-audio_mode.i2s_5rates = 1;
+ } else if ((cfg EM28XX_CHIPCFG_AUDIOMASK) ==
+ EM28XX_CHIPCFG_I2S_3_SAMPRATES) {
+ em28xx_info(I2S Audio (3 sample rates)\n);
+ dev-audio_mode.i2s_3rates = 1;
}
if ((cfg EM28XX_CHIPCFG_AUDIOMASK) != EM28XX_CHIPCFG_AC97) {
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/4] [media] em28xx: usb power config is in the low byte.
According to the em2860 datasheet, eeprom byte 08H is Chip
Configuration Low Byte and 09H is High Byte.
Usb power configuration is in the Low byte (same as the usb audio
class config).
Signed-off-by: Alban Browaeys pra...@yahoo.com
---
drivers/media/usb/em28xx/em28xx-i2c.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index c4ff973..6ff7415 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -743,13 +743,13 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned
bus,
break;
}
- if (le16_to_cpu(dev_config-chip_conf) 1 3)
+ if (le16_to_cpu(dev_config-chip_conf) 4 1 3)
em28xx_info(\tUSB Remote wakeup capable\n);
- if (le16_to_cpu(dev_config-chip_conf) 1 2)
+ if (le16_to_cpu(dev_config-chip_conf) 4 1 2)
em28xx_info(\tUSB Self power capable\n);
- switch (le16_to_cpu(dev_config-chip_conf) 0x3) {
+ switch (le16_to_cpu(dev_config-chip_conf) 4 0x3) {
case 0:
em28xx_info(\t500mA max power\n);
break;
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] [media] em28xx: Fix vidioc fmt vid cap v4l2 compliance
Set fmt.pix.priv to zero in vidioc_g_fmt_vid_cap and vidioc_try_fmt_vid_cap. Catched by v4l2-compliance. Signed-off-by: Alban Browaeys pra...@yahoo.com --- drivers/media/usb/em28xx/em28xx-video.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c index 1a577ed..42930a4 100644 --- a/drivers/media/usb/em28xx/em28xx-video.c +++ b/drivers/media/usb/em28xx/em28xx-video.c @@ -943,6 +943,8 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv, else f-fmt.pix.field = dev-interlaced ? V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP; + f-fmt.pix.priv = 0; + return 0; } @@ -1008,6 +1010,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv, else f-fmt.pix.field = dev-interlaced ? V4L2_FIELD_INTERLACED : V4L2_FIELD_TOP; + f-fmt.pix.priv = 0; return 0; } -- 1.8.3.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: thoughts on kernel security issues
Bill Davidsen tmr.com> writes: > > With no disrespect, I don't believe you have ever been a full-time > employee system administrator for any commercial or government > organization, and I don't believe you have any experience trying to do > security when change must be reviewed by technically naive management to > justify cost, time, and policy implications. The people on the list who > disagree may view the security information issue in a very different > context. Basically you are saying that if i disagree, my view is irrelevant. What do you expect with this kind of start. > Linus Torvalds wrote: > > > What vendor-sec does is to make it "socially acceptable" to be a parasite. > > > > I personally think that such behaviour simply should not be encouraged. If > > you have a security "researcher" that has some reason to delay his > > disclosure, you should see for for what he is: looking for cheap PR. You > > shouldn't make excuses for it. Any research organization that sees PR as a > > primary objective is just misguided. > > There are damn fine reasons for not having immediate public disclosure, > it allows vandors and administrators to close the hole before the script > kiddies get a hold of it. And they are the real problem, because there > are so MANY of them, and they tend to do slash and burn stuff, wipe out > your files, steal your identity, and other things you have to notice. > They aren't smart enough to find holes themselves in most cases, they > are too lazy in many cases to read the high-level hacker boards, and a > few weeks of delay in many cases lets the careful avoid damage. > > Security through obscurity doesn't work, but a small delay for a fix to > be developed can prevent a lot of problems. And of course the > information should be released, it encourages the creation and > installation of fixes. > > Oh, and many of the problem reports result in "cheap PR" consisting of a > single line mention in a CERT report or similar. Most people are not > doing it for the glory. Nobody told against a small delay , in most of the case that is already what is happening today. There is a little problem in this rhetoric. You want fix fast and disclosure latter. As soon as the fix (we are talking about source available) is out, the hole is too. Wondering if chiken or egg is great flame subject. > > > What's the alternative? I'd like to foster a culture of > > > > (a) accepting that bugs happen, and that they aren't news, but making > > sure that the very openness of the process means that people know > > what's going on exactly because it is _open_, not because some news > > organization had to make a big stink about it just to make a vendor > > take notice. > > Linux vendors aside, many vendors react in direct proportion to the bad > publicity engendered. I'd like the world to work that way, but in many > places it doesn't. > > > > Right now, people seem to think that big news media warnings on > > cnet.com about SP2 fixing 15 vulnerabilities or similar is the proper > > way to get people to upgrade. That just -cannot- be right. > > Unfortunately reality doesn't agree with you. Many organizations have no > other effective way to convince management of the need for a fix except > newspaper articles and magazine articles. A sometimes that has to get to > the horror story stage before action is possible. All those to lines to say one thing . Managing security requires social skills. > > And let's not kid ourselves: the security firms may have resources that > > they put into it, but the worst-case schenario is actual criminal intent. > > People who really have resources to study security problems, and who have > > _no_ advantage of using vendor-sec at all. And in that case, vendor-sec is > > _REALLY_ a huge mistake. > > I think you are still missing the point, I don't care if a security firm > reads mailing lists or tea leaves, does research or just knows where to > find it, they are paid to do it and if they do it well and report the > problems which apply to me and the source of the fixes they keep me from > missing something and at the same time save me time. Even reading only > good mailing lists and newsgroups it takes a lot of time to keep > current, and you see a lot of stuff you don't need. > Does this resume to : I want my company to be in control. And nobody else please, because i do not trust them. Who would you want in this security board ? No hackers i believe they have no incentive to shut the *** up, they do not care about money or their buisness or who knows why. So you want : a/ everyboddy is wrong, we cannot understand, b/ crackers "are too lazy in many cases to read the high-level hacker boards" c/ "How can i have fix without ever having a hole ?". Close your eyes and believe, that s the only way to achieved absolute safety. I am not kidding, billions of people does this, it seems
Re: thoughts on kernel security issues
Bill Davidsen davidsen at tmr.com writes: With no disrespect, I don't believe you have ever been a full-time employee system administrator for any commercial or government organization, and I don't believe you have any experience trying to do security when change must be reviewed by technically naive management to justify cost, time, and policy implications. The people on the list who disagree may view the security information issue in a very different context. Basically you are saying that if i disagree, my view is irrelevant. What do you expect with this kind of start. Linus Torvalds wrote: What vendor-sec does is to make it socially acceptable to be a parasite. I personally think that such behaviour simply should not be encouraged. If you have a security researcher that has some reason to delay his disclosure, you should see for for what he is: looking for cheap PR. You shouldn't make excuses for it. Any research organization that sees PR as a primary objective is just misguided. There are damn fine reasons for not having immediate public disclosure, it allows vandors and administrators to close the hole before the script kiddies get a hold of it. And they are the real problem, because there are so MANY of them, and they tend to do slash and burn stuff, wipe out your files, steal your identity, and other things you have to notice. They aren't smart enough to find holes themselves in most cases, they are too lazy in many cases to read the high-level hacker boards, and a few weeks of delay in many cases lets the careful avoid damage. Security through obscurity doesn't work, but a small delay for a fix to be developed can prevent a lot of problems. And of course the information should be released, it encourages the creation and installation of fixes. Oh, and many of the problem reports result in cheap PR consisting of a single line mention in a CERT report or similar. Most people are not doing it for the glory. Nobody told against a small delay , in most of the case that is already what is happening today. There is a little problem in this rhetoric. You want fix fast and disclosure latter. As soon as the fix (we are talking about source available) is out, the hole is too. Wondering if chiken or egg is great flame subject. What's the alternative? I'd like to foster a culture of (a) accepting that bugs happen, and that they aren't news, but making sure that the very openness of the process means that people know what's going on exactly because it is _open_, not because some news organization had to make a big stink about it just to make a vendor take notice. Linux vendors aside, many vendors react in direct proportion to the bad publicity engendered. I'd like the world to work that way, but in many places it doesn't. Right now, people seem to think that big news media warnings on cnet.com about SP2 fixing 15 vulnerabilities or similar is the proper way to get people to upgrade. That just -cannot- be right. Unfortunately reality doesn't agree with you. Many organizations have no other effective way to convince management of the need for a fix except newspaper articles and magazine articles. A sometimes that has to get to the horror story stage before action is possible. All those to lines to say one thing . Managing security requires social skills. And let's not kid ourselves: the security firms may have resources that they put into it, but the worst-case schenario is actual criminal intent. People who really have resources to study security problems, and who have _no_ advantage of using vendor-sec at all. And in that case, vendor-sec is _REALLY_ a huge mistake. I think you are still missing the point, I don't care if a security firm reads mailing lists or tea leaves, does research or just knows where to find it, they are paid to do it and if they do it well and report the problems which apply to me and the source of the fixes they keep me from missing something and at the same time save me time. Even reading only good mailing lists and newsgroups it takes a lot of time to keep current, and you see a lot of stuff you don't need. Does this resume to : I want my company to be in control. And nobody else please, because i do not trust them. Who would you want in this security board ? No hackers i believe they have no incentive to shut the *** up, they do not care about money or their buisness or who knows why. So you want : a/ everyboddy is wrong, we cannot understand, b/ crackers are too lazy in many cases to read the high-level hacker boards c/ How can i have fix without ever having a hole ?. Close your eyes and believe, that s the only way to achieved absolute safety. I am not kidding, billions of people does this, it seems efficient (only few dies by accident). d/ the world is mad , nobody cares about security except
Re: Linux Kernel Audit Project?
John Richard Moser comcast.net> writes: > Is there an official Linux Kernel Audit Project to actively and > aggressively security audit all patches going into the Linux Kernel, or > do they just get a cursory scan for bugs and obvious screwups? >From a user point of view , there is at least tracking: patches are signed and approved by the "component" maintenair before reaching mainline. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Linux Kernel Audit Project?
John Richard Moser nigelenki at comcast.net writes: Is there an official Linux Kernel Audit Project to actively and aggressively security audit all patches going into the Linux Kernel, or do they just get a cursory scan for bugs and obvious screwups? From a user point of view , there is at least tracking: patches are signed and approved by the component maintenair before reaching mainline. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
