* Re: [kernel-hardening] Re: [PATCH v17 09/15] seccomp: remove duplicated failure logging
From: Kees Cook @ 2012-04-09 19:39 UTC (permalink / raw)
To: Eric Paris
Cc: kernel-hardening, Andrew Morton, linux-kernel,
linux-security-module, linux-arch, linux-doc, netdev, x86, arnd,
davem, hpa, mingo, oleg, peterz, rdunlap, mcgrathr, tglx, luto,
serge.hallyn, djm, scarybeasts, indan, pmoore, corbet,
eric.dumazet, markus, coreyb, jmorris
In-Reply-To: <1333999988.14260.9.camel@localhost>
On Mon, Apr 9, 2012 at 12:33 PM, Eric Paris <eparis@redhat.com> wrote:
> On Mon, 2012-04-09 at 14:26 -0500, Will Drewry wrote:
>> On Fri, Apr 6, 2012 at 4:14 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> > On Thu, 29 Mar 2012 15:01:54 -0500
>> > Will Drewry <wad@chromium.org> wrote:
>
>> >> -void __audit_seccomp(unsigned long syscall)
>> >> +void __audit_seccomp(unsigned long syscall, long signr, int code)
>> >> {
>> >> struct audit_buffer *ab;
>> >>
>> >> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
>> >> - audit_log_abend(ab, "seccomp", SIGKILL);
>> >> + audit_log_abend(ab, "seccomp", signr);
>> >> audit_log_format(ab, " syscall=%ld", syscall);
>> >> +#ifdef CONFIG_COMPAT
>> >> + audit_log_format(ab, " compat=%d", is_compat_task());
>> >> +#endif
>> >
>> > We don't need the ifdef for compilation reasons now.
>> >
>> > The question is: should we emit the compat= record on
>> > non-compat-capable architectures? Doing so would be safer - making it
>> > conditional invites people to write x86-only usersapce.
>>
>> I'd certainly prefer it always being there for exactly that reason.
>>
>> Kees, Eric, any preferences? Unless I hear one, I'll just drop the
>> ifdefs in the next revision.
>
> I'd just leave it in unconditionally. The audit parse libraries would
> handle it just fine, but that doesn't mean everyone uses that tool to
> parse the text.
Related to this, can we get this patch into a tree as well?
https://lkml.org/lkml/2012/3/23/332
Thanks,
-Kees
--
Kees Cook
ChromeOS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" 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 v17 15/15] Documentation: prctl/seccomp_filter
From: Will Drewry @ 2012-04-09 19:46 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, corbet, eric.dumazet, markus, coreyb,
keescook, jmorris
In-Reply-To: <20120406142644.223ffff0.akpm@linux-foundation.org>
On Fri, Apr 6, 2012 at 4:26 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Thu, 29 Mar 2012 15:02:00 -0500
> Will Drewry <wad@chromium.org> wrote:
>
>> Documents how system call filtering using Berkeley Packet
>> Filter programs works and how it may be used.
>> Includes an example for x86 and a semi-generic
>> example using a macro-based code generator.
>>
>>
>> ...
>>
>> +Adding architecture support
>> +-----------------------
>> +
>> +See arch/Kconfig for the authoritative requirements. In general, if an
>> +architecture supports both ptrace_event and seccomp, it will be able to
>> +support seccomp filter with minor fixup: SIGSYS support and seccomp return
>> +value checking. Then it must just add CONFIG_HAVE_ARCH_SECCOMP_FILTER
>> +to its arch-specific Kconfig.
>> diff --git a/samples/Makefile b/samples/Makefile
>> index 2f75851..5ef08bb 100644
>> --- a/samples/Makefile
>> +++ b/samples/Makefile
>
> Oh good, I was going to ask about that.
>
> Can we get this code into tools/testing/selftests? That way people
> will run it more often and it's more likely to be maintained as the
> code evolves.
I'm currently using a lightweight testsuite in addition to the
samples. It's a little more oriented at pass/fail behavior. Would it
be more appropriate to post those in addition to, or instead of,
samples?
thanks!
^ permalink raw reply
* Re: 3.2.8/amd64 full interrupt hangs and deadlocks under big network copies (page allocation failure)
From: Marc MERLIN @ 2012-04-09 19:46 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ,
bhutchings-s/n/eUQHGBpZroRs9YW3xA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120409.153452.1284163346306246866.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Mon, Apr 09, 2012 at 03:34:52PM -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Mon, 09 Apr 2012 21:11:12 +0200
>
> > I think Marc posted stack traces showing problem on transmit side.
> ...
> > I dont really understand how it can happen, with MTU=1500
>
> Depending upon the configuration and the driver, wireless can need
> more headroom. For encryption an extra 8 bytes are necessary, and the
> driver may request a variable amount of extra headroom via
> ->hw.extra_tx_headroom
>
> What wireless device are we dealing with again?
Intel(R) Wireless WiFi Link AGN driver for Linux, in-tree:
Copyright(c) 2003-2011 Intel Corporation
iwlwifi 0000:03:00.0: PCI INT A -> GSI 17 (level, low) -> IRQ 17
iwlwifi 0000:03:00.0: setting latency timer to 64
iwlwifi 0000:03:00.0: pci_resource_len = 0x00002000
iwlwifi 0000:03:00.0: pci_resource_base = ffffc900057a4000
iwlwifi 0000:03:00.0: HW Revision ID = 0x0
iwlwifi 0000:03:00.0: irq 47 for MSI/MSI-X
iwlwifi 0000:03:00.0: Detected Intel(R) Ultimate N WiFi Link 5300 AGN, REV=0x24
iwlwifi 0000:03:00.0: L1 Disabled; Enabling L0S
iwlwifi 0000:03:00.0: device EEPROM VER=0x11e, CALIB=0x4
iwlwifi 0000:03:00.0: Device SKU: 0Xf0
iwlwifi 0000:03:00.0: Tunable channels: 13 802.11bg, 24 802.11a channels
along with iwlwifi-5000-5.ucode
Marc
--
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
.... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v17 08/15] seccomp: add system call filtering using BPF
From: Will Drewry @ 2012-04-09 19:59 UTC (permalink / raw)
To: Indan Zupancic
Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
djm, scarybeasts, pmoore, corbet, eric.dumazet, markus, coreyb,
keescook, jmorris
In-Reply-To: <67e30a0c8655fc53a92e8138bba9de66.squirrel@webmail.greenhost.nl>
On Sun, Apr 8, 2012 at 1:22 PM, Indan Zupancic <indan@nul.nu> wrote:
> On Sat, April 7, 2012 06:23, Andrew Morton wrote:
>> hm, I'm surprised that we don't have a zero-returning implementation of
>> is_compat_task() when CONFIG_COMPAT=n. Seems silly. Blames Arnd.
>
> It's sneakily hidden at the end of compat.h.
>
>>> +/**
>>> + * get_u32 - returns a u32 offset into data
>>> + * @data: a unsigned 64 bit value
>>> + * @index: 0 or 1 to return the first or second 32-bits
>>> + *
>>> + * This inline exists to hide the length of unsigned long.
>>> + * If a 32-bit unsigned long is passed in, it will be extended
>>> + * and the top 32-bits will be 0. If it is a 64-bit unsigned
>>> + * long, then whatever data is resident will be properly returned.
>>> + */
>>> +static inline u32 get_u32(u64 data, int index)
>>> +{
>>> + return ((u32 *)&data)[index];
>>> +}
>>
>> This seems utterly broken on big-endian machines. If so: fix. If not:
>> add comment explaining why?
>
> It's not a bug, it's intentional.
>
> I tried to convince them to have a stable ABI for all archs, but they
> didn't want to make the ABI endianness independent, because it looks
> uglier. The argument being that system call numbers are arch dependent
> anyway.
>
> So a filter for a little endian arch needs to check a different offset
> than one for a big endian archs.
Awkward, but in practice it doesn't seem to matter either way --
especially since properly written filters should check the @arch which
indicates the calling convention, endianness, etc.
>>> +static u32 seccomp_run_filters(int syscall)
>>> +{
>>> + struct seccomp_filter *f;
>>> + u32 ret = SECCOMP_RET_KILL;
>>> + /*
>>> + * All filters are evaluated in order of youngest to oldest. The lowest
>>> + * BPF return value always takes priority.
>>> + */
>>
>> The youngest-first design surprised me. It wasn't mentioned at all in
>> the changelog. Thinking about it, I guess it just doesn't matter. But
>> some description of the reasons for and implications of this decision
>> for the uninitiated would be welcome.
>
> I think it's less confusing to not mention the order at all, exactly
> because it doesn't matter. It has been like this from the start, so
> that's why it's not mentioned in the changelog I guess.
Good call - I will remove that comment. The only relevant information
is that the lowest return value wins. I did add a comment up near
struct seccomp_filter with my attempt at explaining the tree
structure, but I didn't detail evaluation order. In this case, it
only is relevant because our only link to the tree is via our local
pointer which happens to be the "youngest".
> The reason to check the youngest first is because the filters are shared
> between processes: childs inherit it. If later on additional filters are
> added, the only way of adding them without modifying an older filter is
> by adding them to the head of the list. This way no locking is needed,
> because filters are only added and removed single-threadedly, and never
> modified when being shared.
I tried a handful of other strategies, but in practice this seemed to
meet the needs with the least complexity/overhead.
>>> +/**
>>> + * seccomp_attach_filter: Attaches a seccomp filter to current.
>>> + * @fprog: BPF program to install
>>> + *
>>> + * Returns 0 on success or an errno on failure.
>>> + */
>>> +static long seccomp_attach_filter(struct sock_fprog *fprog)
>>> +{
>>> + struct seccomp_filter *filter;
>>> + unsigned long fp_size = fprog->len * sizeof(struct sock_filter);
>>> + unsigned long total_insns = fprog->len;
>>> + long ret;
>>> +
>>> + if (fprog->len == 0 || fprog->len > BPF_MAXINSNS)
>>> + return -EINVAL;
>>> +
>>> + for (filter = current->seccomp.filter; filter; filter = filter->prev)
>>> + total_insns += filter->len + 4; /* include a 4 instr penalty */
>>
>> So tasks don't share filters? We copy them by value at fork? Do we do
>> this at vfork() too?
>
> Yes they do. But shared filters are never modified, except for the refcount.
>
>>
>>> + if (total_insns > MAX_INSNS_PER_PATH)
>>> + return -ENOMEM;
>>> +
>>> + /*
>>> + * Installing a seccomp filter requires that the task have
>>> + * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
>>> + * This avoids scenarios where unprivileged tasks can affect the
>>> + * behavior of privileged children.
>>> + */
>>> + if (!current->no_new_privs &&
>>> + security_capable_noaudit(current_cred(), current_user_ns(),
>>> + CAP_SYS_ADMIN) != 0)
>>> + return -EACCES;
>>> +
>>> + /* Allocate a new seccomp_filter */
>>> + filter = kzalloc(sizeof(struct seccomp_filter) + fp_size, GFP_KERNEL);
>>
>> I think this gives userspace an easy way of causing page allocation
>> failure warnings, by permitting large kmalloc() attempts. Add
>> __GFP_NOWARN?
>
> Max is 32kb. sk_attach_filter() in net/core/filter.c is worse,
> it allocates up to 512kb before even checking the length.
>
> What about using GFP_USER (and adding __GFP_NOWARN to GFP_USER) instead?
It looks like GFP_USER|__GFP_NOWARN would make sense here. I'll change it.
>>> + /* Check and rewrite the fprog via the skb checker */
>>> + ret = sk_chk_filter(filter->insns, filter->len);
>>> + if (ret)
>>> + goto fail;
>>> +
>>> + /* Check and rewrite the fprog for seccomp use */
>>> + ret = seccomp_chk_filter(filter->insns, filter->len);
>>
>> "check" is spelled "check"!
>
> Yes, it is and he did spell "check" as "Check".
>
> seccomp_chk_filter() mirrors sk_chk_filter(). So it refers to
> "chk", not "check".
I can change it to be written out or leave it matching the networking
code. Any preferences?
Thanks!
will
^ permalink raw reply
* Re: [PATCH v2 2/2] cgroup: get rid of populate for memcg
From: Glauber Costa @ 2012-04-09 20:18 UTC (permalink / raw)
To: Tejun Heo
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A, Li Zefan, Johannes Weiner,
Michal Hocko, Balbir Singh
In-Reply-To: <20120409181331.GD7522-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 04/09/2012 03:13 PM, Tejun Heo wrote:
> Hello,
>
> On Mon, Apr 09, 2012 at 02:51:10PM -0300, Glauber Costa wrote:
>> On 04/09/2012 02:40 PM, Tejun Heo wrote:
>>> which BTW seems incorrect even on its
>>> own - unmounting and mounting again would probably make the same
>>> notifier registered multiple times corrupting notification chain, and
>>> ref inc on the parent.
>>
>>
>> For the maintainers: Should I fix those in a new submission, or do
>> you intend to do it yourselves?
>>
>> the refcnt dropping should probably be done in my patch, it is a new
>> leak (sorry). The hotplug notifier, as tejun pointed, was already
>> there.
>>
>> It seems simple enough to fix, so if you guys want, I can bundle it in
>> a new submission.
>
> I think it would be best to create a separate patch which is routed
> through the usual memcg path (I suppose memcg patches go through
> -mm?).
>
> Thanks.
>
Tejun,
After debugging this a bit, I found most of it not to be a problem.
Unless I am *very* wrong (and I both read and tested stuff), mount
operations do not recreate the root cgroup. So stuff like the hotcpu
notifier, etc, won't be a problem.
Now, you are definitely correct in pointing out that we start leaking
stuff now - my bad.
But I guess I can then bundle it in a new submission, after shuffling
around stuff a bit. It is really not a bug now, so no reason to route it
separately.
^ permalink raw reply
* [PATCH net-next] rtnetlink: fix spelling errors
From: Stephen Hemminger @ 2012-04-09 20:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Minor spelling mistakes.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/include/net/rtnetlink.h 2012-02-27 08:43:02.400935781 -0800
+++ b/include/net/rtnetlink.h 2012-04-09 13:24:02.705034830 -0700
@@ -41,7 +41,7 @@ static inline int rtnl_msg_family(const
* @get_size: Function to calculate required room for dumping device
* specific netlink attributes
* @fill_info: Function to dump device specific netlink attributes
- * @get_xstats_size: Function to calculate required room for dumping devic
+ * @get_xstats_size: Function to calculate required room for dumping device
* specific statistics
* @fill_xstats: Function to dump device specific statistics
*/
@@ -94,7 +94,7 @@ extern void rtnl_link_unregister(struct
* @fill_link_af: Function to fill IFLA_AF_SPEC with address family
* specific netlink attributes.
* @get_link_af_size: Function to calculate size of address family specific
- * netlink attributes exlusive the container attribute.
+ * netlink attributes exclusive the container attribute.
* @validate_link_af: Validate a IFLA_AF_SPEC attribute, must check attr
* for invalid configuration settings.
* @set_link_af: Function to parse a IFLA_AF_SPEC attribute and modify
^ permalink raw reply
* Re: [PATCH v2 2/2] cgroup: get rid of populate for memcg
From: Tejun Heo @ 2012-04-09 20:39 UTC (permalink / raw)
To: Glauber Costa
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A, Li Zefan, Johannes Weiner,
Michal Hocko, Balbir Singh
In-Reply-To: <4F83442C.8040006-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Hello, Glauber.
On Mon, Apr 09, 2012 at 05:18:52PM -0300, Glauber Costa wrote:
> After debugging this a bit, I found most of it not to be a problem.
> Unless I am *very* wrong (and I both read and tested stuff), mount
> operations do not recreate the root cgroup. So stuff like the hotcpu
> notifier, etc, won't be a problem.
Ooh, right, root css is created during subsys init. Please note that
the kernel will panic on subsys init failure (hmm....).
> Now, you are definitely correct in pointing out that we start
> leaking stuff now - my bad.
>
> But I guess I can then bundle it in a new submission, after
> shuffling around stuff a bit. It is really not a bug now, so no
> reason to route it separately.
Sure, just make it a separate patch. As long as memcg ppl are fine
with it, I can route the patches together.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Markus Gutschke @ 2012-04-09 20:47 UTC (permalink / raw)
To: Will Drewry
Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
djm, scarybeasts, indan, pmoore, corbet, eric.dumazet, coreyb,
keescook, jmorris
In-Reply-To: <CABqD9haT5h-7LMkbGSaQbnk8SJj9L21GT61UZCMVWLJCTw9OBg@mail.gmail.com>
No matter what you do, please leave the samples accessible somewhere.
They proved incredibly useful in figuring out how the API works. I am
sure, other developers are going to appreciate them as well.
Alternatively, if you don't want to include the samples with the
kernel sources, figure out how you can include a sample in the
official manual page for prctl().
Markus
^ permalink raw reply
* [PATCH 04/26] typhoon: get rid of redundant conditional before all to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, David Dillow
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
There's no need to test for a NULL pointer before calling
release_firmware() since the function does that check itself, so
remove the redundant test.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/3com/typhoon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 1234a14..b153666 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2549,8 +2549,7 @@ typhoon_init(void)
static void __exit
typhoon_cleanup(void)
{
- if (typhoon_fw)
- release_firmware(typhoon_fw);
+ release_firmware(typhoon_fw);
pci_unregister_driver(&typhoon_driver);
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 05/26] tg3: remove redundant NULL test before release_firmware() call
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, Michael Chan, Matt Carlson
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
There is no need to test for a NULL pointer before calling
release_firmware - the function does that on its own.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/broadcom/tg3.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 062ac33..9fbf73e 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -15842,8 +15842,7 @@ static void __devexit tg3_remove_one(struct pci_dev *pdev)
if (dev) {
struct tg3 *tp = netdev_priv(dev);
- if (tp->fw)
- release_firmware(tp->fw);
+ release_firmware(tp->fw);
tg3_reset_task_cancel(tp);
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 06/26] bna: remove redundant NULL test before release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, Rasesh Mody
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
release_firmware() does its own NULL test so explicit test before call
is unneeded.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/brocade/bna/bnad.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index ff78f77..d86390c 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -3546,9 +3546,7 @@ static void __exit
bnad_module_exit(void)
{
pci_unregister_driver(&bnad_pci_driver);
-
- if (bfi_fw)
- release_firmware(bfi_fw);
+ release_firmware(bfi_fw);
}
module_init(bnad_module_init);
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 07/26] qlogic, netxen: get rid of a redundant test for NULL before call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, Rajesh Borundia, Sony Chacko
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
Since release_firmware() deals gracefully with being passed a NULL
pointer there is no reason to test explicitly before calling the
function.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 718b274..1fb149c 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -1261,8 +1261,7 @@ next:
void
netxen_release_firmware(struct netxen_adapter *adapter)
{
- if (adapter->fw)
- release_firmware(adapter->fw);
+ release_firmware(adapter->fw);
adapter->fw = NULL;
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 08/26] qlogic, qlcnic: get rid of redundant test for NULL before a call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel
Cc: trivial, netdev, linux-driver, Sony Chacko, Anirban Chakraborty
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
Since release_firmware() deals gracefully with being passed a NULL
pointer there's no reason to test explicitly before calling the
function.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index d32cf0d..799fd40 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1321,8 +1321,7 @@ next:
void
qlcnic_release_firmware(struct qlcnic_adapter *adapter)
{
- if (adapter->fw)
- release_firmware(adapter->fw);
+ release_firmware(adapter->fw);
adapter->fw = NULL;
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 09/26] tehuti: delete redundant NULL check before release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, Andy Gospodarek
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
release_firmware() checks for NULL pointers - no need to test before
the call.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/ethernet/tehuti/tehuti.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/tehuti/tehuti.c b/drivers/net/ethernet/tehuti/tehuti.c
index ad973ff..a445e77 100644
--- a/drivers/net/ethernet/tehuti/tehuti.c
+++ b/drivers/net/ethernet/tehuti/tehuti.c
@@ -341,8 +341,8 @@ static int bdx_fw_load(struct bdx_priv *priv)
out:
if (master)
WRITE_REG(priv, regINIT_SEMAPHORE, 1);
- if (fw)
- release_firmware(fw);
+
+ release_firmware(fw);
if (rc) {
netdev_err(priv->ndev, "firmware loading failed\n");
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 10/26] wireless, at76c50x:: Don't needlessly test for NULL before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, linux-wireless, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
The release_firmware() function deals gracefully with being passed a
NULL pointer, so explicit tests before the call are rather pointless.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/at76c50x-usb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/at76c50x-usb.c b/drivers/net/wireless/at76c50x-usb.c
index 4045e5a..faa8bcb 100644
--- a/drivers/net/wireless/at76c50x-usb.c
+++ b/drivers/net/wireless/at76c50x-usb.c
@@ -2512,10 +2512,8 @@ static void __exit at76_mod_exit(void)
printk(KERN_INFO DRIVER_DESC " " DRIVER_VERSION " unloading\n");
usb_deregister(&at76_driver);
- for (i = 0; i < ARRAY_SIZE(firmwares); i++) {
- if (firmwares[i].fw)
- release_firmware(firmwares[i].fw);
- }
+ for (i = 0; i < ARRAY_SIZE(firmwares); i++)
+ release_firmware(firmwares[i].fw);
led_trigger_unregister_simple(ledtrig_tx);
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 11/26] wireless, atmel: remove pointless test for NULL before release_firmware() call
From: Jesper Juhl @ 2012-04-09 20:50 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: trivial-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, John W. Linville,
Simon Kelley
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925-h2p7t3/P30RzeRGmFJ5qR7ZzlVVXadcDXqFh9Ls21Oc@public.gmane.org>
release_firmware() does its own test. Explicitly checking before the
call is redundant.
Signed-off-by: Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org>
---
drivers/net/wireless/atmel.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/atmel.c b/drivers/net/wireless/atmel.c
index 6c87a82..d07c030 100644
--- a/drivers/net/wireless/atmel.c
+++ b/drivers/net/wireless/atmel.c
@@ -3989,8 +3989,7 @@ static int reset_atmel_card(struct net_device *dev)
atmel_copy_to_card(priv->dev, 0x8000, &fw[0x6000], len - 0x6000);
}
- if (fw_entry)
- release_firmware(fw_entry);
+ release_firmware(fw_entry);
}
err = atmel_wakeup_firmware(priv);
--
1.7.10
--
Jesper Juhl <jj-IYz4IdjRLj0sV2N9l4h3zg@public.gmane.org> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 12/26] ipw2200: remove a redundant NULL check before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: trivial, netdev, linux-wireless, John W. Linville,
Stanislav Yakovlev
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
The release_firmware() function does its own NULL test, so testing
before calling it is rather redundant.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/ipw2x00/ipw2200.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c
index 2b02257..77c5d2f 100644
--- a/drivers/net/wireless/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/ipw2x00/ipw2200.c
@@ -3657,8 +3657,7 @@ static int ipw_load(struct ipw_priv *priv)
priv->rxq = NULL;
}
ipw_tx_queue_free(priv);
- if (raw)
- release_firmware(raw);
+ release_firmware(raw);
#ifdef CONFIG_PM
fw_loaded = 0;
raw = NULL;
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 13/26] wireless, libertas: remove redundant NULL tests before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: trivial, netdev, linux-wireless, libertas-dev, John W. Linville,
Dan Williams
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
release_firmware() tests for, and deals gracefully with, NULL
pointers. Remove redundant explicit tests before calling the function.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/libertas/if_cs.c | 6 ++----
drivers/net/wireless/libertas/if_sdio.c | 6 ++----
drivers/net/wireless/libertas/if_spi.c | 6 ++----
drivers/net/wireless/libertas/main.c | 12 ++++--------
4 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/net/wireless/libertas/if_cs.c b/drivers/net/wireless/libertas/if_cs.c
index 234ee88..171a06b 100644
--- a/drivers/net/wireless/libertas/if_cs.c
+++ b/drivers/net/wireless/libertas/if_cs.c
@@ -951,10 +951,8 @@ out2:
out1:
pcmcia_disable_device(p_dev);
out:
- if (helper)
- release_firmware(helper);
- if (mainfw)
- release_firmware(mainfw);
+ release_firmware(helper);
+ release_firmware(mainfw);
lbs_deb_leave_args(LBS_DEB_CS, "ret %d", ret);
return ret;
diff --git a/drivers/net/wireless/libertas/if_sdio.c b/drivers/net/wireless/libertas/if_sdio.c
index 9804ebc..15bfe2f 100644
--- a/drivers/net/wireless/libertas/if_sdio.c
+++ b/drivers/net/wireless/libertas/if_sdio.c
@@ -751,10 +751,8 @@ success:
ret = 0;
out:
- if (helper)
- release_firmware(helper);
- if (mainfw)
- release_firmware(mainfw);
+ release_firmware(helper);
+ release_firmware(mainfw);
lbs_deb_leave_args(LBS_DEB_SDIO, "ret %d", ret);
return ret;
diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index 50b1ee7..7a5df4f 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -1095,10 +1095,8 @@ static int if_spi_init_card(struct if_spi_card *card)
goto out;
out:
- if (helper)
- release_firmware(helper);
- if (mainfw)
- release_firmware(mainfw);
+ release_firmware(helper);
+ release_firmware(mainfw);
lbs_deb_leave_args(LBS_DEB_SPI, "err %d\n", err);
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 957681d..3b81b70 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -1269,14 +1269,10 @@ int lbs_get_firmware(struct device *dev, const char *user_helper,
fail:
/* Failed */
- if (*helper) {
- release_firmware(*helper);
- *helper = NULL;
- }
- if (*mainfw) {
- release_firmware(*mainfw);
- *mainfw = NULL;
- }
+ release_firmware(*helper);
+ *helper = NULL;
+ release_firmware(*mainfw);
+ *mainfw = NULL;
return -ENOENT;
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 14/26] wireless, mwifiex: drop redundant NULL test before call to release_firmware()
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
To: linux-kernel; +Cc: trivial, netdev, linux-wireless, John W. Linville, Bing Zhao
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
Since release_firmware() does its own test for NULL it is redundant to
do so before calling it.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/mwifiex/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index 9d1b3ca..2ee6162 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -342,8 +342,7 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
ret = 0;
done:
- if (adapter->firmware)
- release_firmware(adapter->firmware);
+ release_firmware(adapter->firmware);
if (ret)
ret = -1;
return ret;
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 15/26] wireless, orinoco: release_firmware() tests for NULL, remove explicit tests before calls
From: Jesper Juhl @ 2012-04-09 20:51 UTC (permalink / raw)
To: linux-kernel
Cc: trivial, netdev, linux-wireless, Pavel Roskin, Paul Gortmaker,
John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
It is redundant to test for NULL pointers before calling
release_firmware() since the function does its own NULL test.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/orinoco/fw.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/orinoco/fw.c b/drivers/net/wireless/orinoco/fw.c
index 4df8cf6..400a352 100644
--- a/drivers/net/wireless/orinoco/fw.c
+++ b/drivers/net/wireless/orinoco/fw.c
@@ -379,11 +379,8 @@ void orinoco_cache_fw(struct orinoco_private *priv, int ap)
void orinoco_uncache_fw(struct orinoco_private *priv)
{
- if (priv->cached_pri_fw)
- release_firmware(priv->cached_pri_fw);
- if (priv->cached_fw)
- release_firmware(priv->cached_fw);
-
+ release_firmware(priv->cached_pri_fw);
+ release_firmware(priv->cached_fw);
priv->cached_pri_fw = NULL;
priv->cached_fw = NULL;
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* [PATCH 26/26] ipw2100: remove a redundant NULL check before calling release_firmware()
From: Jesper Juhl @ 2012-04-09 20:52 UTC (permalink / raw)
To: linux-kernel
Cc: trivial, netdev, linux-wireless, John W. Linville,
Stanislav Yakovlev
In-Reply-To: <alpine.LNX.2.00.1204092157340.13925@swampdragon.chaosbits.net>
The release_firmware() function does its own NULL test so a test
before calling it is rather redundant.
Signed-off-by: Jesper Juhl <jj@chaosbits.net>
---
drivers/net/wireless/ipw2x00/ipw2100.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
index f0551f8..d8d804e 100644
--- a/drivers/net/wireless/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/ipw2x00/ipw2100.c
@@ -8508,8 +8508,7 @@ static void ipw2100_release_firmware(struct ipw2100_priv *priv,
struct ipw2100_fw *fw)
{
fw->version = 0;
- if (fw->fw_entry)
- release_firmware(fw->fw_entry);
+ release_firmware(fw->fw_entry);
fw->fw_entry = NULL;
}
--
1.7.10
--
Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
^ permalink raw reply related
* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Ryan Ware @ 2012-04-09 20:58 UTC (permalink / raw)
To: Markus Gutschke, Will Drewry
Cc: Andrew Morton, linux-kernel, linux-security-module, linux-arch,
linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
djm, scarybeasts, indan, pmoore, corbet, eric.dumazet, coreyb,
keescook, jmorris
In-Reply-To: <CAE6n16kMv5Y+Cp9XZ=tmK0sgoJJZ6CJUq_ttuNEUMb4JFXJ33g@mail.gmail.com>
I second this! They are extremely useful.
Ryan
On 4/9/12 1:47 PM, "Markus Gutschke" <markus@chromium.org> wrote:
>No matter what you do, please leave the samples accessible somewhere.
>They proved incredibly useful in figuring out how the API works. I am
>sure, other developers are going to appreciate them as well.
>
>Alternatively, if you don't want to include the samples with the
>kernel sources, figure out how you can include a sample in the
>official manual page for prctl().
>
>
>Markus
>--
>To unsubscribe from this list: send the line "unsubscribe
>linux-security-module" 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 14/26] wireless, mwifiex: drop redundant NULL test before call to release_firmware()
From: Bing Zhao @ 2012-04-09 20:59 UTC (permalink / raw)
To: Jesper Juhl, linux-kernel@vger.kernel.org
Cc: trivial@kernel.org, netdev@vger.kernel.org,
linux-wireless@vger.kernel.org, John W. Linville
In-Reply-To: <alpine.LNX.2.00.1204092224080.13925@swampdragon.chaosbits.net>
Hi Jesper,
Thanks for the patch.
> Since release_firmware() does its own test for NULL it is redundant to
> do so before calling it.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Acked-by: Bing Zhao <bzhao@marvell.com>
Thanks,
Bing
> ---
> drivers/net/wireless/mwifiex/main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
> index 9d1b3ca..2ee6162 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -342,8 +342,7 @@ static int mwifiex_init_hw_fw(struct mwifiex_adapter *adapter)
> ret = 0;
>
> done:
> - if (adapter->firmware)
> - release_firmware(adapter->firmware);
> + release_firmware(adapter->firmware);
> if (ret)
> ret = -1;
> return ret;
> --
> 1.7.10
>
>
> --
> Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/
> Don't top-post http://www.catb.org/jargon/html/T/top-post.html
> Plain text mails only, please.
^ permalink raw reply
* Re: [PATCH 04/26] typhoon: get rid of redundant conditional before all to release_firmware()
From: David Dillow @ 2012-04-09 21:03 UTC (permalink / raw)
To: Jesper Juhl; +Cc: linux-kernel, trivial, netdev
In-Reply-To: <alpine.LNX.2.00.1204092208140.13925@swampdragon.chaosbits.net>
On Mon, 2012-04-09 at 22:50 +0200, Jesper Juhl wrote:
> There's no need to test for a NULL pointer before calling
> release_firmware() since the function does that check itself, so
> remove the redundant test.
>
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
FWIW on a trivial patch,
Acked-by: David Dillow <dave@thedillows.org>
^ permalink raw reply
* RE: [PATCH 06/26] bna: remove redundant NULL test before release_firmware()
From: Rasesh Mody @ 2012-04-09 21:14 UTC (permalink / raw)
To: Jesper Juhl, linux-kernel@vger.kernel.org
Cc: trivial@kernel.org, netdev@vger.kernel.org
In-Reply-To: <alpine.LNX.2.00.1204092210470.13925@swampdragon.chaosbits.net>
>From: Jesper Juhl [mailto:jj@chaosbits.net]
>Sent: Monday, April 09, 2012 1:50 PM
>To: linux-kernel@vger.kernel.org
>
>release_firmware() does its own NULL test so explicit test before call
>is unneeded.
>
>Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Thanks Jesper!
Acked-by: Rasesh Mody <rmody@brocade.com>
^ 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