* Re: [PATCH 2/2] e1000: remove risky prefetch on next_skb->data
From: Andi Kleen @ 2006-06-06 0:22 UTC (permalink / raw)
To: Rick Jones
Cc: Brandeburg, Jesse, Kok, Auke-jan H, Garzik, Jeff, netdev,
Kok, Auke
In-Reply-To: <4484C95B.6010009@hp.com>
> Bear with me, I'm a software guy :) I interpret that to mean that the
> processor is basically broken? If so, wouldn't it be the case that
> prefetch() needs to become a noop on that processor?
I would agree with Rick - if prefetch() is broken on Xscale it should be disabled
in the architecture, not in individual drivers. Otherwise all other code
that use it might be broken too.
Maybe it's only broken on MMIO and not normal memory - in that
case introducing a mmio_prefetch() (and defining it as an nop on Xscale) would be also
an option.
-Andi
^ permalink raw reply
* Re: [PATCH 2/2] e1000: remove risky prefetch on next_skb->data
From: Brandeburg, Jesse @ 2006-06-06 0:26 UTC (permalink / raw)
To: Rick Jones
Cc: Brandeburg, Jesse, Kok, Auke-jan H, Garzik, Jeff, netdev,
Kok, Auke
In-Reply-To: <4484C95B.6010009@hp.com>
On Mon, 5 Jun 2006, Rick Jones wrote:
> Brandeburg, Jesse wrote:
> > Hi Rick, according to our reporter, receives break. The prefetch (not
> > always, but sometimes) lets the processor get junk from the prefetched
> > area. Apparently this version of arm doesn't quite do as strict
> > enforcement of bus snoops as x86, ia64, (and even pSeries!) does.
>
> Bear with me, I'm a software guy :) I interpret that to mean that the
> processor is basically broken? If so, wouldn't it be the case that
> prefetch() needs to become a noop on that processor?
For a software guy, you're making a large leap :-) I wouldn't say the
processor is broken, it is more sensitive to our (admittedly) bad behavior
when prefetching data for one descriptor *past* any that we currently
*know* are done. ARM/XSCALE is able to use prefetch all over the place in
its arch specific code with no problems (i'm relying on advice from
someone else on this, i haven't validated with mine own eyes)
Someone else had complained about this particular prefetch anyway (and it
was the most speculative with the least amount of gain) on the list
before, so given this information we're trying to take the conservative
route.
This patch was tested by the reporter and he was pleased with the result.
> > This manifested with a large drop in receive peformance using TCP,
> > probably because it was retransmitting frequently.
>
> I forget - what were the gains on the other CPUs?
One system (from a while back) I have numbers for showed a 10% increase in
packets per second that could be handled using netperf udp receive, with
the prefetch code in place.
And to Andi's make that came in while I was typing this, I reiterate I do
not believe ARM/XSCALE prefetch to be broken.
Jesse
^ permalink raw reply
* Re: [PATCH 2.6.18 2/3] tg3: Convert to non-LLTX
From: Stephen Hemminger @ 2006-06-06 0:31 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, herbert, jgarzik, netdev
In-Reply-To: <1149542985.13155.11.camel@rh4>
On Mon, 05 Jun 2006 14:29:45 -0700
"Michael Chan" <mchan@broadcom.com> wrote:
> On Mon, 2006-06-05 at 15:58 -0700, Stephen Hemminger wrote:
>
> >
> > Since you are going more lockless, you probably need memory barriers.
>
> No, we're not going more lockless. We're simply replacing the private
> tx_lock with dev->xmit_lock by dropping the LLTX feature flag. The
> amount of locking is exactly the same as before.
Those spin lock's were also acting as barriers.
Are you sure code is safe in the face of cpu reordering.
--
If one would give me six lines written by the hand of the most honest
man, I would find something in them to have him hanged. -- Cardinal Richlieu
^ permalink raw reply
* Re: [PATCH 2.6.18 2/3] tg3: Convert to non-LLTX
From: Michael Chan @ 2006-06-05 23:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: davem, herbert, jgarzik, netdev
In-Reply-To: <20060605173159.33d81d59@localhost.localdomain>
On Mon, 2006-06-05 at 17:31 -0700, Stephen Hemminger wrote:
> On Mon, 05 Jun 2006 14:29:45 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
>
> > On Mon, 2006-06-05 at 15:58 -0700, Stephen Hemminger wrote:
> >
> > >
> > > Since you are going more lockless, you probably need memory barriers.
> >
> > No, we're not going more lockless. We're simply replacing the private
> > tx_lock with dev->xmit_lock by dropping the LLTX feature flag. The
> > amount of locking is exactly the same as before.
>
> Those spin lock's were also acting as barriers.
Why is it different whether we use dev->xmit_lock right before entering
hard_start_xmit() without LLTX or a private tx_lock at the beginning of
hard_start_xmit() with LLTX? In both cases, we take one BH disabling
lock to serialize hard_start_xmit(). And in both cases, the spinlock
will provide the same barrier effect.
> Are you sure code is safe in the face of cpu reordering.
>
Yes, hard_start_xmit() only touches the tx producer index and tx
completion handling only touches the tx consumer index.
There is actually one wrinkle but it has nothing to do with the locking
change in this patch. During the sequence of writing the tx descriptors
to memory and the MMIO write to tell the chip to DMA the new
descriptors, the MMIO can be re-ordered ahead of the writing of the tx
descriptors on the powerpc, for example. This will lead to DMAing stale
descriptor data. In earlier kernels, the ppc writel has a memory barrier
to prevent this but it was removed in recent kernels. We had a
discussion with Anton Blanchard and other folks at IBM and DaveM on this
and it was decided that the writel should provide the barrier instead of
drivers adding all kinds of barriers throughout the driver code.
^ permalink raw reply
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
From: Jeff Garzik @ 2006-06-06 1:18 UTC (permalink / raw)
To: Greg KH
Cc: Jiri Slaby, Greg KH, Linux Kernel Mailing List, linux-pci,
jgarzik, netdev, mb, st3, linville
In-Reply-To: <20060605205309.GA31061@kroah.com>
On Mon, Jun 05, 2006 at 01:53:09PM -0700, Greg KH wrote:
> Why not just use the proper pci interface? Why poke around in another
> pci device to steal an irq, when that irq might not even be valid?
> (irqs are not valid until pci_enable_device() is called on them...)
Answered this question the last time you asked.
Answer: this is an embedded platform that needs such poking. The
wireless device is _another_ device.
Jeff
^ permalink raw reply
* Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock
From: Herbert Xu @ 2006-06-06 2:04 UTC (permalink / raw)
To: Michael Chan; +Cc: davem, jgarzik, netdev
In-Reply-To: <1149536843.13155.6.camel@rh4>
On Mon, Jun 05, 2006 at 12:47:23PM -0700, Michael Chan wrote:
> Remove tx_lock where it is unnecessary. tg3 runs lockless and so it
> requires interrupts to be disabled and sync'ed, netif_queue and NAPI
> poll to be stopped before the device can be reconfigured. After
> stopping everything, it is no longer necessary to get the tx_lock.
The paths where full lock is preceded by netif_tx_disable are obviously
safe (well, once you take off the LLTX flag anyway). However, there are
paths that don't do netif_tx_disable (e.g., tg3_set_rx_mode), are they
safe as well?
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] hush noisy ieee80211 CCMP printks
From: Jouni Malinen @ 2006-06-06 3:41 UTC (permalink / raw)
To: Jason Lunz; +Cc: linville, netdev
In-Reply-To: <20060605160335.GB25688@knob.reflex>
On Mon, Jun 05, 2006 at 12:03:35PM -0400, Jason Lunz wrote:
> If these are a real problem, I'll gladly help debug it. I'm using
> 2.6.17-rc5's bcm43xx and the in-kernel ieee80211 softmac stack. I'm
> using wpa/ccmp/aes with an openwrt AP.
Do you happen to have a wireless sniffer that you could use to capture
the frames? It would be interesting to see whether such a capture log
could be mapped into the dropped frames shown in the kernel debug log.
> > In many cases, this may be caused by something missing in local
> > filtering (e.g., retry duplicates are not filtered out correctly or
> > messages to incorrect addresses are allowed through, etc.).
>
> Is it the driver's responsibility to do this filtering, or the softmac
> stack's? Can you show me a code example where this filtering is done
> properly?
This is generic functionality and something I would like to see
implemented in the 802.11 stack so that every low-level driver would not
need to do this. I don't remember what net/ieee80211 code does in this
area. Devicescape code (net/d80211 in wireless-dev.git) has duplicate
detection in ieee80211_rx_h_check().
> I can try different implementations of WPA in the AP if necessary. I
> think openwrt has other options. My end is in-kernel ieee80211_softmac -
> is there anything you'd like me to look at there?
Would you be interested in testing this with net/d80211 code and
wireless-dev.git? It would be interesting to see whether you would get a
different result with the AP end kept in identical configuration and
just changing the client side 802.11 network stack. With net/d80211, the
CCMP replay detection printk is disabled by default (which is what
should be done with net/ieee80211 implementation, too), so that needs to
be enabled at build time. Counters are available from procfs files,
though, so number of replays can be compared.
> If the messages are actually useful, and unusual, then they should
> probably stay. But I get a LOT of them, and my wireless is working just
> fine.
Unfortunately, they are not really unusual. They are useful--at least
for developers--so #ifdef or run-time option for enabling/disabling them
would be nice.
> I noticed that there are counters being incremented where those printks
> are. How do I see those counters from userspace?
These used to be visible from procfs in the original implementation in
Host AP driver. However, it looks like print_stats function is not
called at all in the version that is in net/ieee80211, so the counters
may not be exposed at the moment. They should be, though..
--
Jouni Malinen PGP id EFC895FA
^ permalink raw reply
* Re: [PATCH] Fixup struct ip_mc_list::multiaddr type
From: David Miller @ 2006-06-06 4:04 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20060526193009.GB7266@martell.zuzino.mipt.ru>
From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Fri, 26 May 2006 23:30:09 +0400
> All users except two expect 32-bit big-endian value. One is of
>
> ->multiaddr = ->multiaddr
>
> variety. And last one is "%08lX".
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Applied, thanks Alexey.
^ permalink raw reply
* Re: [PATCH] Right prototype of __raw_v4_lookup()
From: David Miller @ 2006-06-06 4:07 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20060527210932.GB7267@martell.zuzino.mipt.ru>
From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Sun, 28 May 2006 01:09:32 +0400
> All users pass 32-bit values as addresses and internally they're compared with
> 32-bit entities. So, change "laddr" and "raddr" types to __be32.
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Applied.
Doesn't this mean that, at least eventually, we should change
the address members of inet_sock to __be32 as well?
^ permalink raw reply
* Re: [PATCH] unsigned long => __be32 in icmp_redirect()
From: David Miller @ 2006-06-06 4:18 UTC (permalink / raw)
To: adobriyan; +Cc: netdev
In-Reply-To: <20060527212925.GC7267@martell.zuzino.mipt.ru>
From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Sun, 28 May 2006 01:29:25 +0400
> ip_rt_redirect() expects __be32 as second arg, so...
>
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
I'm going to cure this by simple killing the 'ip' variable
altogether. It is assigned once, to iph->daddr, and used
once, as an argument. So just pass iph->daddr directly.
Thanks for finding this.
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: David Miller @ 2006-06-06 4:32 UTC (permalink / raw)
To: herbert; +Cc: mchan, jgarzik, netdev
In-Reply-To: <20060601111503.GA16097@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 1 Jun 2006 21:15:04 +1000
> On Thu, Jun 01, 2006 at 10:25:25AM +1000, herbert wrote:
> >
> > > I think this netpoll wrinkle means we also have to make
> > > sure to set the xmit_lock_owner across the board.
> >
> > You're right. In fact this can deadlock today for those drivers that
> > already make use of xmit_lock without setting the owner. So I suppose
> > something like net_xmit_lock to obtain xmit_lock is called for.
>
> OK, here is a patch which does this.
>
> [NET]: Add netif_tx_lock
IPOIB is going to BUG() with this change. Because now, in their
multicast code, you're going to local_bh_disable() via
netif_tx_unlock() with hw IRQs disabled which is illegal.
It shows a bug here in the locking of the IPOIB driver.
We need to think about this change some more. :)
^ permalink raw reply
* [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: Catherine Zhang @ 2006-06-06 4:38 UTC (permalink / raw)
To: James Morris, netdev, davem, chrisw, herbert, sds, tjaeger
Cc: latten, sergeh, gcwilson, czhang.us
Hi,
Minor fix per James' comment.
thanks,
Catherine
--
This patch contains a fix for the previous patch that adds security
contexts to IPsec policies and security associations. In the previous
patch, no authorization (besides the check for write permissions to
SAD and SPD) is required to delete IPsec policies and security
assocations with security contexts. Thus a user authorized to change
SAD and SPD can bypass the IPsec policy authorization by simply
deleteing policies with security contexts. To fix this security hole,
an additional authorization check is added for removing security
policies and security associations with security contexts.
Note that if no security context is supplied on add or present on
policy to be deleted, the SELinux module allows the change
unconditionally. The hook is called on deletion when no context is
present, which we may want to change. At present, I left it up to the
module.
LSM changes:
The patch adds two new LSM hooks: xfrm_policy_delete and
xfrm_state_delete. The new hooks are necessary to authorize deletion
of IPsec policies that have security contexts. The existing hooks
xfrm_policy_free and xfrm_state_free lack the context to do the
authorization, so I decided to split authorization of deletion and
memory management of security data, as is typical in the LSM
interface.
Use:
The new delete hooks are checked when xfrm_policy or xfrm_state are
deleted by either the xfrm_user interface (xfrm_get_policy,
xfrm_del_sa) or the pfkey interface (pfkey_spddelete, pfkey_delete).
SELinux changes:
The new policy_delete and state_delete functions are added.
Signed-off-by: Trent Jaeger <tjaeger@cse.psu.edu>
---
include/linux/security.h | 40 ++++++++++++++++++++++++++++++++++------
net/key/af_key.c | 17 +++++++++++------
net/xfrm/xfrm_user.c | 19 ++++++++++++-------
security/dummy.c | 12 ++++++++++++
security/selinux/hooks.c | 2 ++
security/selinux/include/xfrm.h | 2 ++
security/selinux/xfrm.c | 39 +++++++++++++++++++++++++++++++++++----
7 files changed, 108 insertions(+), 23 deletions(-)
diff -puN include/linux/security.h~lsm-secpeer-fix include/linux/security.h
--- linux-2.6.17-rc4-mm3/include/linux/security.h~lsm-secpeer-fix 2006-05-31 00:01:35.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/include/linux/security.h 2006-05-31 00:46:13.000000000 -0400
@@ -805,31 +805,37 @@ struct swap_info_struct;
* used by the XFRM system.
* @sec_ctx contains the security context information being provided by
* the user-level policy update program (e.g., setkey).
- * Allocate a security structure to the xp->selector.security field.
+ * Allocate a security structure to the xp->security field.
* The security field is initialized to NULL when the xfrm_policy is
* allocated.
* Return 0 if operation was successful (memory to allocate, legal context)
* @xfrm_policy_clone_security:
* @old contains an existing xfrm_policy in the SPD.
* @new contains a new xfrm_policy being cloned from old.
- * Allocate a security structure to the new->selector.security field
- * that contains the information from the old->selector.security field.
+ * Allocate a security structure to the new->security field
+ * that contains the information from the old->security field.
* Return 0 if operation was successful (memory to allocate).
* @xfrm_policy_free_security:
* @xp contains the xfrm_policy
- * Deallocate xp->selector.security.
+ * Deallocate xp->security.
+ * @xfrm_policy_delete_security:
+ * @xp contains the xfrm_policy.
+ * Authorize deletion of xp->security.
* @xfrm_state_alloc_security:
* @x contains the xfrm_state being added to the Security Association
* Database by the XFRM system.
* @sec_ctx contains the security context information being provided by
* the user-level SA generation program (e.g., setkey or racoon).
- * Allocate a security structure to the x->sel.security field. The
+ * Allocate a security structure to the x->security field. The
* security field is initialized to NULL when the xfrm_state is
* allocated.
* Return 0 if operation was successful (memory to allocate, legal context).
* @xfrm_state_free_security:
* @x contains the xfrm_state.
- * Deallocate x>sel.security.
+ * Deallocate x->security.
+ * @xfrm_state_delete_security:
+ * @x contains the xfrm_state.
+ * Authorize deletion of x->security.
* @xfrm_policy_lookup:
* @xp contains the xfrm_policy for which the access control is being
* checked.
@@ -1298,8 +1304,10 @@ struct security_operations {
int (*xfrm_policy_alloc_security) (struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx);
int (*xfrm_policy_clone_security) (struct xfrm_policy *old, struct xfrm_policy *new);
void (*xfrm_policy_free_security) (struct xfrm_policy *xp);
+ int (*xfrm_policy_delete_security) (struct xfrm_policy *xp);
int (*xfrm_state_alloc_security) (struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx);
void (*xfrm_state_free_security) (struct xfrm_state *x);
+ int (*xfrm_state_delete_security) (struct xfrm_state *x);
int (*xfrm_policy_lookup)(struct xfrm_policy *xp, u32 sk_sid, u8 dir);
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
@@ -2934,11 +2942,21 @@ static inline void security_xfrm_policy_
security_ops->xfrm_policy_free_security(xp);
}
+static inline int security_xfrm_policy_delete(struct xfrm_policy *xp)
+{
+ return security_ops->xfrm_policy_delete_security(xp);
+}
+
static inline int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx)
{
return security_ops->xfrm_state_alloc_security(x, sec_ctx);
}
+static inline int security_xfrm_state_delete(struct xfrm_state *x)
+{
+ return security_ops->xfrm_state_delete_security(x);
+}
+
static inline void security_xfrm_state_free(struct xfrm_state *x)
{
security_ops->xfrm_state_free_security(x);
@@ -2963,6 +2981,11 @@ static inline void security_xfrm_policy_
{
}
+static inline int security_xfrm_policy_delete(struct xfrm_policy *xp)
+{
+ return 0;
+}
+
static inline int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx)
{
return 0;
@@ -2972,6 +2995,11 @@ static inline void security_xfrm_state_f
{
}
+static inline int security_xfrm_state_delete(struct xfrm_policy *xp)
+{
+ return 0;
+}
+
static inline int security_xfrm_policy_lookup(struct xfrm_policy *xp, u32 sk_sid, u8 dir)
{
return 0;
diff -puN net/key/af_key.c~lsm-secpeer-fix net/key/af_key.c
--- linux-2.6.17-rc4-mm3/net/key/af_key.c~lsm-secpeer-fix 2006-05-31 00:01:46.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/net/key/af_key.c 2006-05-31 00:04:22.000000000 -0400
@@ -1454,21 +1454,23 @@ static int pfkey_delete(struct sock *sk,
if (x == NULL)
return -ESRCH;
+ if ((err = security_xfrm_state_delete(x)))
+ goto out;
+
if (xfrm_state_kern(x)) {
- xfrm_state_put(x);
- return -EPERM;
+ err = -EPERM;
+ goto out;
}
err = xfrm_state_delete(x);
- if (err < 0) {
- xfrm_state_put(x);
- return err;
- }
+ if (err < 0)
+ goto out;
c.seq = hdr->sadb_msg_seq;
c.pid = hdr->sadb_msg_pid;
c.event = XFRM_MSG_DELSA;
km_state_notify(x, &c);
+out:
xfrm_state_put(x);
return err;
@@ -2274,11 +2276,14 @@ static int pfkey_spddelete(struct sock *
err = 0;
+ if ((err = security_xfrm_policy_delete(xp)))
+ goto out;
c.seq = hdr->sadb_msg_seq;
c.pid = hdr->sadb_msg_pid;
c.event = XFRM_MSG_DELPOLICY;
km_policy_notify(xp, pol->sadb_x_policy_dir-1, &c);
+out:
xfrm_pol_put(xp);
return err;
}
diff -puN net/xfrm/xfrm_user.c~lsm-secpeer-fix net/xfrm/xfrm_user.c
--- linux-2.6.17-rc4-mm3/net/xfrm/xfrm_user.c~lsm-secpeer-fix 2006-05-31 00:01:54.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/net/xfrm/xfrm_user.c 2006-05-31 00:04:22.000000000 -0400
@@ -427,23 +427,25 @@ static int xfrm_del_sa(struct sk_buff *s
if (x == NULL)
return -ESRCH;
+ if (err = security_xfrm_state_delete(x))
+ goto out;
+
if (xfrm_state_kern(x)) {
- xfrm_state_put(x);
- return -EPERM;
+ err = -EPERM;
+ goto out;
}
err = xfrm_state_delete(x);
- if (err < 0) {
- xfrm_state_put(x);
- return err;
- }
+ if (err < 0)
+ goto out;
c.seq = nlh->nlmsg_seq;
c.pid = nlh->nlmsg_pid;
c.event = nlh->nlmsg_type;
km_state_notify(x, &c);
- xfrm_state_put(x);
+out:
+ xfrm_state_put(x);
return err;
}
@@ -1055,6 +1057,8 @@ static int xfrm_get_policy(struct sk_buf
MSG_DONTWAIT);
}
} else {
+ if (err = security_xfrm_policy_delete(xp))
+ goto out;
c.data.byid = p->index;
c.event = nlh->nlmsg_type;
c.seq = nlh->nlmsg_seq;
@@ -1064,6 +1068,7 @@ static int xfrm_get_policy(struct sk_buf
xfrm_pol_put(xp);
+out:
return err;
}
diff -puN security/dummy.c~lsm-secpeer-fix security/dummy.c
--- linux-2.6.17-rc4-mm3/security/dummy.c~lsm-secpeer-fix 2006-05-31 00:02:04.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/security/dummy.c 2006-05-31 00:04:22.000000000 -0400
@@ -810,6 +810,11 @@ static void dummy_xfrm_policy_free_secur
{
}
+static int dummy_xfrm_policy_delete_security(struct xfrm_policy *xp)
+{
+ return 0;
+}
+
static int dummy_xfrm_state_alloc_security(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx)
{
return 0;
@@ -819,6 +824,11 @@ static void dummy_xfrm_state_free_securi
{
}
+static int dummy_xfrm_state_delete_security(struct xfrm_state *x)
+{
+ return 0;
+}
+
static int dummy_xfrm_policy_lookup(struct xfrm_policy *xp, u32 sk_sid, u8 dir)
{
return 0;
@@ -1024,8 +1034,10 @@ void security_fixup_ops (struct security
set_to_dummy_if_null(ops, xfrm_policy_alloc_security);
set_to_dummy_if_null(ops, xfrm_policy_clone_security);
set_to_dummy_if_null(ops, xfrm_policy_free_security);
+ set_to_dummy_if_null(ops, xfrm_policy_delete_security);
set_to_dummy_if_null(ops, xfrm_state_alloc_security);
set_to_dummy_if_null(ops, xfrm_state_free_security);
+ set_to_dummy_if_null(ops, xfrm_state_delete_security);
set_to_dummy_if_null(ops, xfrm_policy_lookup);
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
#ifdef CONFIG_KEYS
diff -puN security/selinux/hooks.c~lsm-secpeer-fix security/selinux/hooks.c
--- linux-2.6.17-rc4-mm3/security/selinux/hooks.c~lsm-secpeer-fix 2006-05-31 00:02:11.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/security/selinux/hooks.c 2006-05-31 00:04:22.000000000 -0400
@@ -4400,8 +4400,10 @@ static struct security_operations selinu
.xfrm_policy_alloc_security = selinux_xfrm_policy_alloc,
.xfrm_policy_clone_security = selinux_xfrm_policy_clone,
.xfrm_policy_free_security = selinux_xfrm_policy_free,
+ .xfrm_policy_delete_security = selinux_xfrm_policy_delete,
.xfrm_state_alloc_security = selinux_xfrm_state_alloc,
.xfrm_state_free_security = selinux_xfrm_state_free,
+ .xfrm_state_delete_security = selinux_xfrm_state_delete,
.xfrm_policy_lookup = selinux_xfrm_policy_lookup,
#endif
};
diff -puN security/selinux/include/xfrm.h~lsm-secpeer-fix security/selinux/include/xfrm.h
--- linux-2.6.17-rc4-mm3/security/selinux/include/xfrm.h~lsm-secpeer-fix 2006-05-31 00:02:19.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/security/selinux/include/xfrm.h 2006-05-31 00:04:22.000000000 -0400
@@ -9,8 +9,10 @@
int selinux_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx);
int selinux_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new);
void selinux_xfrm_policy_free(struct xfrm_policy *xp);
+int selinux_xfrm_policy_delete(struct xfrm_policy *xp);
int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx);
void selinux_xfrm_state_free(struct xfrm_state *x);
+int selinux_xfrm_state_delete(struct xfrm_state *x);
int selinux_xfrm_policy_lookup(struct xfrm_policy *xp, u32 sk_sid, u8 dir);
/*
diff -puN security/selinux/xfrm.c~lsm-secpeer-fix security/selinux/xfrm.c
--- linux-2.6.17-rc4-mm3/security/selinux/xfrm.c~lsm-secpeer-fix 2006-05-31 00:02:28.000000000 -0400
+++ linux-2.6.17-rc4-mm3-cxzhang/security/selinux/xfrm.c 2006-06-06 00:24:37.000000000 -0400
@@ -132,10 +132,7 @@ static int selinux_xfrm_sec_ctx_alloc(st
goto out;
/*
- * Does the subject have permission to set security or permission to
- * do the relabel?
- * Must be permitted to relabel from default socket type (process type)
- * to specified context
+ * Does the subject have permission to set security context?
*/
rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
SECCLASS_ASSOCIATION,
@@ -201,6 +198,23 @@ void selinux_xfrm_policy_free(struct xfr
}
/*
+ * LSM hook implementation that authorizes deletion of labeled policies.
+ */
+int selinux_xfrm_policy_delete(struct xfrm_policy *xp)
+{
+ struct task_security_struct *tsec = current->security;
+ struct xfrm_sec_ctx *ctx = xp->security;
+ int rc = 0;
+
+ if (ctx)
+ rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+ SECCLASS_ASSOCIATION,
+ ASSOCIATION__SETCONTEXT, NULL);
+
+ return rc;
+}
+
+/*
* LSM hook implementation that allocs and transfers sec_ctx spec to
* xfrm_state.
*/
@@ -292,6 +306,23 @@ u32 selinux_socket_getpeer_dgram(struct
return SECSID_NULL;
}
+ /*
+ * LSM hook implementation that authorizes deletion of labeled SAs.
+ */
+int selinux_xfrm_state_delete(struct xfrm_state *x)
+{
+ struct task_security_struct *tsec = current->security;
+ struct xfrm_sec_ctx *ctx = x->security;
+ int rc = 0;
+
+ if (ctx)
+ rc = avc_has_perm(tsec->sid, ctx->ctx_sid,
+ SECCLASS_ASSOCIATION,
+ ASSOCIATION__SETCONTEXT, NULL);
+
+ return rc;
+}
+
/*
* LSM hook that controls access to unlabelled packets. If
* a xfrm_state is authorizable (defined by macro) then it was
_
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Roland Dreier @ 2006-06-06 4:44 UTC (permalink / raw)
To: David Miller; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <20060605.213250.85688925.davem@davemloft.net>
> IPOIB is going to BUG() with this change. Because now, in their
> multicast code, you're going to local_bh_disable() via
> netif_tx_unlock() with hw IRQs disabled which is illegal.
>
> It shows a bug here in the locking of the IPOIB driver.
Sorry, I haven't followed this thread closely. Can you expand on what
the bug in ipoib's multicast locking is?
Thanks,
Roland
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Roland Dreier @ 2006-06-06 4:50 UTC (permalink / raw)
To: David Miller; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adapshmnbam.fsf@cisco.com>
Roland> Sorry, I haven't followed this thread closely. Can you
Roland> expand on what the bug in ipoib's multicast locking is?
You know, looking at the ipoib code, I can't even recreate why
xmit_lock is taken in the set_multicast_list method anyway, or how it
works at all -- it seems set_multicast_list will always be called with
xmit_lock already held. What the heck is going on?
- R.
^ permalink raw reply
* Re: [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: James Morris @ 2006-06-06 4:51 UTC (permalink / raw)
To: Catherine Zhang
Cc: netdev, davem, chrisw, herbert, sds, tjaeger, latten, sergeh,
gcwilson, czhang.us
In-Reply-To: <20060606043818.GA4125@jiayuguan.watson.ibm.com>
On Tue, 6 Jun 2006, Catherine Zhang wrote:
> Minor fix per James' comment.
Acked-by: James Morris <jmorris@namei.org>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: David Miller @ 2006-06-06 4:57 UTC (permalink / raw)
To: rdreier; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adapshmnbam.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 05 Jun 2006 21:44:01 -0700
> > IPOIB is going to BUG() with this change. Because now, in their
> > multicast code, you're going to local_bh_disable() via
> > netif_tx_unlock() with hw IRQs disabled which is illegal.
> >
> > It shows a bug here in the locking of the IPOIB driver.
>
> Sorry, I haven't followed this thread closely. Can you expand on what
> the bug in ipoib's multicast locking is?
It disables hw interrupts, and takes the xmit_lock.
xmit_lock is a BH only spinlock, you can't take it from hw
interrupts, or inside of a HW irq protected spinlock.
You can take it on the outside of a HW irq lock protected
area like this:
spin_lock_bh(&dev->xmit_lock);
spin_lock_irq(&whatever_lock);
spin_unlock_irq(&whatever_lock);
spin_unlock_bh(&dev->xmit_lock);
but not the other way around.
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Roland Dreier @ 2006-06-06 4:57 UTC (permalink / raw)
To: David Miller; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adalksanb0c.fsf@cisco.com>
Roland> You know, looking at the ipoib code, I can't even recreate
Roland> why xmit_lock is taken in the set_multicast_list method
Roland> anyway, or how it works at all -- it seems
Roland> set_multicast_list will always be called with xmit_lock
Roland> already held. What the heck is going on?
Never mind -- I see that the set_multicast_list work needs to be
deferred to process context, so ipoib_mcast_restart_task() doesn't run
directly from the call to set_multicast_list.
I guess the fix in the current kernel is just something like the
below. And in the netif_tx_lock() patch, the local_irq_save() /
local_irq_restore() calls can just be removed.
Am I on the right track?
Anyway I won't push the patch below since the bug is harmless right
now and it can be fixed up as part of the netif_tx_lock() patch.
Thanks,
Roland
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index ec41c8f..5f3eaf1 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -814,13 +814,12 @@ void ipoib_mcast_restart_task(void *dev_
struct dev_mc_list *mclist;
struct ipoib_mcast *mcast, *tmcast;
LIST_HEAD(remove_list);
- unsigned long flags;
ipoib_dbg_mcast(priv, "restarting multicast task\n");
ipoib_mcast_stop_thread(dev, 0);
- spin_lock_irqsave(&dev->xmit_lock, flags);
+ spin_lock_bh(&dev->xmit_lock);
spin_lock(&priv->lock);
/*
@@ -895,7 +894,7 @@ void ipoib_mcast_restart_task(void *dev_
}
spin_unlock(&priv->lock);
- spin_unlock_irqrestore(&dev->xmit_lock, flags);
+ spin_unlock_bh(&dev->xmit_lock);
/* We have to cancel outside of the spinlock */
list_for_each_entry_safe(mcast, tmcast, &remove_list, list) {
^ permalink raw reply related
* Re: [NET]: Add netif_tx_lock
From: David Miller @ 2006-06-06 4:58 UTC (permalink / raw)
To: rdreier; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adalksanb0c.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 05 Jun 2006 21:50:11 -0700
> Roland> Sorry, I haven't followed this thread closely. Can you
> Roland> expand on what the bug in ipoib's multicast locking is?
>
> You know, looking at the ipoib code, I can't even recreate why
> xmit_lock is taken in the set_multicast_list method anyway, or how it
> works at all -- it seems set_multicast_list will always be called with
> xmit_lock already held. What the heck is going on?
Isn't the IPOIB driver LLTX and therefore the upper layers are
not taking the xmit_lock?
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Roland Dreier @ 2006-06-06 5:04 UTC (permalink / raw)
To: David Miller; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <20060605.215821.69020274.davem@davemloft.net>
David> Isn't the IPOIB driver LLTX and therefore the upper layers
David> are not taking the xmit_lock?
Yeah, but I think that should be OK. If I'm remembering the intention
of the code correctly, the reason xmit_lock is being taken there is
just to protect dev->mc_list -- and this is needed because ipoib has
to defer handling set_multicast_list to process context. Am I right
in thinking that dev->xmit_lock still serializes mc_list operations,
even for LLTX drivers?
- R.
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: David Miller @ 2006-06-06 5:09 UTC (permalink / raw)
To: rdreier; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adad5dmnacd.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 05 Jun 2006 22:04:34 -0700
> Am I right in thinking that dev->xmit_lock still serializes mc_list
> operations, even for LLTX drivers?
Correct.
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: David Miller @ 2006-06-06 5:10 UTC (permalink / raw)
To: rdreier; +Cc: herbert, mchan, jgarzik, netdev
In-Reply-To: <adahd2ynank.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 05 Jun 2006 21:57:51 -0700
> Roland> You know, looking at the ipoib code, I can't even recreate
> Roland> why xmit_lock is taken in the set_multicast_list method
> Roland> anyway, or how it works at all -- it seems
> Roland> set_multicast_list will always be called with xmit_lock
> Roland> already held. What the heck is going on?
>
> Never mind -- I see that the set_multicast_list work needs to be
> deferred to process context, so ipoib_mcast_restart_task() doesn't run
> directly from the call to set_multicast_list.
>
> I guess the fix in the current kernel is just something like the
> below. And in the netif_tx_lock() patch, the local_irq_save() /
> local_irq_restore() calls can just be removed.
>
> Am I on the right track?
>
> Anyway I won't push the patch below since the bug is harmless right
> now and it can be fixed up as part of the netif_tx_lock() patch.
As long as you never take priv->lock while ->xmit_lock is held
your patch should be OK.
^ permalink raw reply
* Re: [PATCH 2.6.18 1/3] tg3: Remove unnecessary tx_lock
From: Michael Chan @ 2006-06-06 5:10 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, jgarzik, netdev
Herbert Xu wrote:
> The paths where full lock is preceded by netif_tx_disable are
> obviously
> safe (well, once you take off the LLTX flag anyway).
> However, there are
> paths that don't do netif_tx_disable (e.g., tg3_set_rx_mode), are they
> safe as well?
>
In places where we don't call tg3_netif_stop() before tg3_full_lock(),
we are typically doing one of the following:
- changing tg3_flags or tg3_flags2
- reprogramming MAC address
- reprogramming interrupt coalescing values
- reprogramming the PHY
- setting the rx mode
The TX code does not do any of these so it is safe.
tg3_set_rx_mode() is actually called with xmit_lock held through
dev->set_multicast_list(). Or it is called after tg3_netif_stop() in
the VLAN registration calls (in my patch #3).
^ permalink raw reply
* Re: Linux kernel and laws
From: Evgeniy Polyakov @ 2006-06-06 5:33 UTC (permalink / raw)
To: Adrian Bunk
Cc: Jeff Garzik, Arjan van de Ven, Christoph Hellwig, Andrew Morton,
linux-kernel, netdev, linville
In-Reply-To: <20060605140226.GR3955@stusta.de>
On Mon, Jun 05, 2006 at 04:02:26PM +0200, Adrian Bunk (bunk@stusta.de) wrote:
> > Far too many people have a careless "U.S.A. laws suck, merge it anyway"
> > attitude.
> If someone would state a submission to the kernel might have issues
> according to Chinese laws, or Iranian laws, or Russian laws, would this
> be enough for keeping code out of the kernel?
Btw, did kernel hackers consulted with Papua New Guinea or bloody
Russian laws? It is possible that they have a law which forbids to write
open source code. So we should stop Linux kernel development and completely
remove it's sources from the Internet ASAP.
P.S. It is explicitly permitted to make reverse engineering in Russia.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: James Morris @ 2006-06-06 5:37 UTC (permalink / raw)
To: Catherine Zhang
Cc: netdev, davem, chrisw, herbert, sds, tjaeger, latten, sergeh,
gcwilson, czhang.us
In-Reply-To: <20060606043818.GA4125@jiayuguan.watson.ibm.com>
On Tue, 6 Jun 2006, Catherine Zhang wrote:
> Minor fix per James' comment.
Can you also add a Signed-off-by line?
I can't recall if you were the original author. If not, we also need a
>From line (per Documentation/SubmittingPatches).
Thanks,
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [linux-usb-devel] [PATCH RFC] ZyDAS ZD1211 USB-WLAN driver
From: David Brownell @ 2006-06-06 5:41 UTC (permalink / raw)
To: linux-usb-devel
Cc: Oliver Neukum, Daniel Drake, netdev, Ulrich Kunitz,
John W. Linville
In-Reply-To: <200606040025.32275.oliver@neukum.org>
On Saturday 03 June 2006 3:25 pm, Oliver Neukum wrote:
> Am Samstag, 3. Juni 2006 21:35 schrieb Daniel Drake:
> > Oliver Neukum wrote:
> > > +static int read_mac_addr(struct zd_chip *chip, u8 *mac_addr)
> > > +{
> > > + static const zd_addr_t addr[2] = { CR_MAC_ADDR_P1, CR_MAC_ADDR_P2 };
> > > + return _read_mac_addr(chip, mac_addr, (const zd_addr_t *)addr);
> > > +}
> > >
> > > Why on the stack?
> >
> > Technically it's not on the stack because it is static const (it goes in
> > rodata), but I don't think that this invalidates your point. What's the
> > alternative? kmalloc and kfree every time?
>
> In this case rodata will work. However, if you ever switch to direct DMA
> it will fail. I really did overlook the const keyword.
On some platforms rodata will work; but it's not guaranteed on any.
See Documentation/DMA-mapping.txt and read the little section at the
top explaining what types of memory can be DMA'd from ... neither stack,
nor BSS, nor data, nor rodata, nor text are OK.
- Dave
^ 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