Netdev List
 help / color / mirror / Atom feed
* Re: [Intel-wired-lan] [PATCH] [v2] i40e: avoid 64-bit division where possible
From: Alexander Duyck @ 2017-10-17 17:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jeff Kirsher, Netdev, Mitch Williams,
	linux-kernel@vger.kernel.org, intel-wired-lan,
	Björn Töpel, Filip Sadowski, David S. Miller
In-Reply-To: <20171017155001.318184-1-arnd@arndb.de>

On Tue, Oct 17, 2017 at 8:49 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> The new bandwidth calculation caused a link error on 32-bit
> architectures, like
>
> ERROR: "__aeabi_uldivmod" [drivers/net/ethernet/intel/i40e/i40e.ko] undefined!
>
> The problem is the max_tx_rate calculation that uses 64-bit integers.
> This is not really necessary since the numbers are in MBit/s so
> they won't be higher than 40000 for the highest support rate, and
> are guaranteed to not exceed 2^32 in future generations either.
>
> Another patch from Alan Brady fixed the link error by adding
> many calls to do_div(), which makes the code less efficent and
> less readable than necessary.
>
> This changes the representation to 'u32' when dealing with MBit/s
> and uses div_u64() to convert from u64 numbers in byte/s, reverting
> parts of Alan's earlier fix that have become obsolete now.
>
> Fixes: 2027d4deacb1 ("i40e: Add support setting TC max bandwidth rates")
> Fixes: 73983b5ae011 ("i40e: fix u64 division usage")
> Cc: Alan Brady <alan.brady@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

So this patch looks good to me, we just need to test it to verify it
doesn't break existing functionality. In the meantime if Alan's patch
has gone through testing we should probably push "i40e: fix u64
division usage" to Dave so that we can at least fix the linking issues
on ARM and i386.

Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>

> ---
>  drivers/net/ethernet/intel/i40e/i40e.h      |  4 +-
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 70 +++++++++++------------------
>  2 files changed, 27 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index c3f13120f3ce..c7aa0c982273 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -407,7 +407,7 @@ struct i40e_channel {
>         u8 enabled_tc;
>         struct i40e_aqc_vsi_properties_data info;
>
> -       u64 max_tx_rate;
> +       u32 max_tx_rate; /* in Mbits/s */
>
>         /* track this channel belongs to which VSI */
>         struct i40e_vsi *parent_vsi;
> @@ -1101,5 +1101,5 @@ static inline bool i40e_enabled_xdp_vsi(struct i40e_vsi *vsi)
>  }
>
>  int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
> -int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate);
> +int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate);
>  #endif /* _I40E_H_ */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 3ceda140170d..57682cc78508 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -5448,17 +5448,16 @@ int i40e_get_link_speed(struct i40e_vsi *vsi)
>   *
>   * Helper function to set BW limit for a given VSI
>   **/
> -int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
> +int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate)
>  {
>         struct i40e_pf *pf = vsi->back;
> -       u64 credits = 0;
>         int speed = 0;
>         int ret = 0;
>
>         speed = i40e_get_link_speed(vsi);
>         if (max_tx_rate > speed) {
>                 dev_err(&pf->pdev->dev,
> -                       "Invalid max tx rate %llu specified for VSI seid %d.",
> +                       "Invalid max tx rate %u specified for VSI seid %d.",
>                         max_tx_rate, seid);
>                 return -EINVAL;
>         }
> @@ -5469,13 +5468,12 @@ int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
>         }
>
>         /* Tx rate credits are in values of 50Mbps, 0 is disabled */
> -       credits = max_tx_rate;
> -       do_div(credits, I40E_BW_CREDIT_DIVISOR);
> -       ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid, credits,
> +       ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid,
> +                                         max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>                                           I40E_MAX_BW_INACTIVE_ACCUM, NULL);
>         if (ret)
>                 dev_err(&pf->pdev->dev,
> -                       "Failed set tx rate (%llu Mbps) for vsi->seid %u, err %s aq_err %s\n",
> +                       "Failed set tx rate (%u Mbps) for vsi->seid %u, err %s aq_err %s\n",
>                         max_tx_rate, seid, i40e_stat_str(&pf->hw, ret),
>                         i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
>         return ret;
> @@ -6158,17 +6156,13 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
>
>         /* configure VSI for BW limit */
>         if (ch->max_tx_rate) {
> -               u64 credits = ch->max_tx_rate;
> -
>                 if (i40e_set_bw_limit(vsi, ch->seid, ch->max_tx_rate))
>                         return -EINVAL;
>
> -               do_div(credits, I40E_BW_CREDIT_DIVISOR);
>                 dev_dbg(&pf->pdev->dev,
> -                       "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
> +                       "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>                         ch->max_tx_rate,
> -                       credits,
> -                       ch->seid);
> +                       ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR, ch->seid);
>         }
>
>         /* in case of VF, this will be main SRIOV VSI */
> @@ -6189,7 +6183,6 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
>  static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
>  {
>         struct i40e_channel *ch;
> -       u64 max_rate = 0;
>         int ret = 0, i;
>
>         /* Create app vsi with the TCs. Main VSI with TC0 is already set up */
> @@ -6211,9 +6204,8 @@ static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
>                         /* Bandwidth limit through tc interface is in bytes/s,
>                          * change to Mbit/s
>                          */
> -                       max_rate = vsi->mqprio_qopt.max_rate[i];
> -                       do_div(max_rate, I40E_BW_MBPS_DIVISOR);
> -                       ch->max_tx_rate = max_rate;
> +                       ch->max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[i],
> +                                                 I40E_BW_CREDIT_DIVISOR);
>
>                         list_add_tail(&ch->list, &vsi->ch_list);
>
> @@ -6643,7 +6635,6 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
>                                      struct tc_mqprio_qopt_offload *mqprio_qopt)
>  {
>         u64 sum_max_rate = 0;
> -       u64 max_rate = 0;
>         int i;
>
>         if (mqprio_qopt->qopt.offset[0] != 0 ||
> @@ -6658,9 +6649,8 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
>                                 "Invalid min tx rate (greater than 0) specified\n");
>                         return -EINVAL;
>                 }
> -               max_rate = mqprio_qopt->max_rate[i];
> -               do_div(max_rate, I40E_BW_MBPS_DIVISOR);
> -               sum_max_rate += max_rate;
> +               sum_max_rate += div_u64(mqprio_qopt->max_rate[i],
> +                                       I40E_BW_CREDIT_DIVISOR);
>
>                 if (i >= mqprio_qopt->qopt.num_tc - 1)
>                         break;
> @@ -6804,18 +6794,14 @@ static int i40e_setup_tc(struct net_device *netdev, void *type_data)
>
>         if (pf->flags & I40E_FLAG_TC_MQPRIO) {
>                 if (vsi->mqprio_qopt.max_rate[0]) {
> -                       u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
> -
> -                       do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
> +                       u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
> +                                                 I40E_BW_CREDIT_DIVISOR);
>                         ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
>                         if (!ret) {
> -                               u64 credits = max_tx_rate;
> -
> -                               do_div(credits, I40E_BW_CREDIT_DIVISOR);
>                                 dev_dbg(&vsi->back->pdev->dev,
> -                                       "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
> +                                       "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>                                         max_tx_rate,
> -                                       credits,
> +                                       max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>                                         vsi->seid);
>                         } else {
>                                 need_reset = true;
> @@ -9024,15 +9010,12 @@ static int i40e_rebuild_channels(struct i40e_vsi *vsi)
>                         return ret;
>                 }
>                 if (ch->max_tx_rate) {
> -                       u64 credits = ch->max_tx_rate;
> -
>                         if (i40e_set_bw_limit(vsi, ch->seid,
>                                               ch->max_tx_rate))
>                                 return -EINVAL;
>
> -                       do_div(credits, I40E_BW_CREDIT_DIVISOR);
>                         dev_dbg(&vsi->back->pdev->dev,
> -                               "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
> +                               "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
>                                 ch->max_tx_rate,
>                                 ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR,
>                                 ch->seid);
> @@ -9314,21 +9297,18 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
>         }
>
>         if (vsi->mqprio_qopt.max_rate[0]) {
> -               u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
> -               u64 credits = 0;
> +               u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
> +                                         I40E_BW_CREDIT_DIVISOR);
>
> -               do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
>                 ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
> -               if (ret)
> +               if (!ret)
> +                       dev_dbg(&vsi->back->pdev->dev,
> +                               "Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
> +                               max_tx_rate,
> +                               max_tx_rate / I40E_BW_CREDIT_DIVISOR,
> +                               vsi->seid);
> +               else
>                         goto end_unlock;
> -
> -               credits = max_tx_rate;
> -               do_div(credits, I40E_BW_CREDIT_DIVISOR);
> -               dev_dbg(&vsi->back->pdev->dev,
> -                       "Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
> -                       max_tx_rate,
> -                       credits,
> -                       vsi->seid);
>         }
>
>         ret = i40e_rebuild_cloud_filters(vsi, vsi->seid);
> --
> 2.9.0
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply

* Re: [PATCH net-next] tcp: Enable TFO without a cookie on a per-socket basis
From: Yuchung Cheng @ 2017-10-17 17:26 UTC (permalink / raw)
  To: Christoph Paasch; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <20171017063714.17346-1-cpaasch@apple.com>

On Mon, Oct 16, 2017 at 11:37 PM, Christoph Paasch <cpaasch@apple.com> wrote:
> We already allow to enable TFO without a cookie by using the
> fastopen-sysctl and setting it to TFO_SERVER_COOKIE_NOT_REQD (0x200).
> This is safe to do in certain environments where we know that there
> isn't a malicous host (aka., data-centers).
>
> A server however might be talking to both sides (public Internet and
> data-center). So, this server would want to enable cookie-less TFO for
> the connections that go to the data-center while enforcing cookies for
> the traffic from the Internet.
>
> This patch exposes a socket-option to enable this (protected by
> CAP_NET_ADMIN).
>
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>
> ---
>  include/linux/tcp.h      |  1 +
>  include/uapi/linux/tcp.h |  1 +
>  net/ipv4/tcp.c           | 14 ++++++++++++++
>  net/ipv4/tcp_fastopen.c  |  6 ++++--
>  4 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 1d2c44e09e31..cda5d4dc8d70 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -228,6 +228,7 @@ struct tcp_sock {
>                 syn_fastopen_ch:1, /* Active TFO re-enabling probe */
>                 syn_data_acked:1,/* data in SYN is acked by SYN-ACK */
>                 save_syn:1,     /* Save headers of SYN packet */
> +               no_tfo_cookie:1, /* Allow send/recv SYN+data without a cookie */
can we rename to fastopen_no_cookie and move one line above so TFO
stuff is together with similar naming.

>                 is_cwnd_limited:1;/* forward progress limited by snd_cwnd? */
>         u32     tlp_high_seq;   /* snd_nxt at the time of TLP retransmit. */
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index 15c25eccab2b..d44f4bef056c 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -119,6 +119,7 @@ enum {
>  #define TCP_FASTOPEN_CONNECT   30      /* Attempt FastOpen with connect */
>  #define TCP_ULP                        31      /* Attach a ULP to a TCP connection */
>  #define TCP_MD5SIG_EXT         32      /* TCP MD5 Signature with extensions */
> +#define TCP_NO_TFO_COOKIE      33      /* Enable TFO without a TFO cookie */
>
>  struct tcp_repair_opt {
>         __u32   opt_code;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 3b34850d361f..88c90be12d9f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2821,6 +2821,16 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>                         err = -EOPNOTSUPP;
>                 }
>                 break;
> +       case TCP_NO_TFO_COOKIE:
rename to TCP_FASTOPEN_NO_COOKIE for better consistency on TFO
options? I am also cooking a TCP_FASTOPEN_KEY option patch to allow
listener to update the key.

> +               if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
> +                       err = -EPERM;
> +               else if (val > 1 || val < 0)
> +                       err = -EINVAL;
> +               else if (!((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)))
> +                       err = -EINVAL;
> +               else
> +                       tp->no_tfo_cookie = 1;
> +               break;
>         case TCP_TIMESTAMP:
>                 if (!tp->repair)
>                         err = -EPERM;
> @@ -3219,6 +3229,10 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
>                 val = tp->fastopen_connect;
>                 break;
>
> +       case TCP_NO_TFO_COOKIE:
> +               val = tp->no_tfo_cookie;
> +               break;
> +
>         case TCP_TIMESTAMP:
>                 val = tcp_time_stamp_raw() + tp->tsoffset;
>                 break;
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 7ee4aadcdd71..c1b00b666b43 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -309,7 +309,8 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
>                 return NULL;
>         }
>
> -       if (syn_data && (tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
> +       if (syn_data && ((tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD) ||
> +                        tcp_sk(sk)->no_tfo_cookie))
>                 goto fastopen;
>
>         if (foc->len >= 0 &&  /* Client presents or requests a cookie */
> @@ -363,7 +364,8 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
>                 return false;
>         }
>
> -       if (sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
> +       if ((sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) ||
> +           tcp_sk(sk)->no_tfo_cookie) {
>                 cookie->len = -1;
>                 return true;
>         }
> --
> 2.14.1
>

^ permalink raw reply

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Marcelo Ricardo Leitner @ 2017-10-17 17:27 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Xin Long, Eric Dumazet, Vlad Yasevich, Neil Horman, netdev,
	Wei Wang, linux-sctp
In-Reply-To: <CANn89iKWrWnoVAu+KXhT-HLnGFme6ojKRCxwS5syrACiGVB1=g@mail.gmail.com>

On Tue, Oct 17, 2017 at 10:20:58AM -0700, Eric Dumazet wrote:
> On Tue, Oct 17, 2017 at 10:01 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Tue, Oct 17, 2017 at 09:44:10AM -0700, Eric Dumazet wrote:
> >> On Tue, Oct 17, 2017 at 9:28 AM, Marcelo Ricardo Leitner
> >> <marcelo.leitner@gmail.com> wrote:
> >> > On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
> >> >> On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >> >> > SCTP experts.
> >> >> >
> >> >> > syszkaller reported a few crashes in sctp_packet_config() with invalid
> >> >> > access to a deleted dst.
> >> >> >
> >> >> > The rcu_read_lock() in sctp_packet_config() is suspect.
> >> >> >
> >> >> > It does not protect anything at the moment.
> >> >> >
> >> >> > If we expect tp->dst to be manipulated/changed by another cpu/thread,
> >> >> > then we need proper rcu protection.
> >> >> >
> >> >> > Following patch to show what would be a minimal change (but obviously
> >> >> > bigger changes are needed, like sctp_transport_pmtu_check() and
> >> >> > sctp_transport_dst_check(), and proper sparse annotations)
> >> >> will check all places accessing tp->dst in sctp.
> >> >
> >> > I checked some and sctp_transport_dst_check() should be fine because
> >> > by then we are holding a reference on dst. Same goes to
> >> > sctp_transport_pmtu_check().
> >>
> >> Really ?
> >>
> >
> > Yes,
> >
> >> What about sctp_v4_err() -> sctp_icmp_redirect() -> sctp_transport_dst_check()
> >>
> >> It seems quite possible that the BH handler can access it, while
> >> socket is owned by user.
> >
> > hidden here:
> > sctp_v4_err() {
> > ...
> >         sk = sctp_err_lookup(net, AF_INET, skb, sctp_hdr(skb), &asoc,
> >         &transport);
> > ...
> > out_unlock:
> >         sctp_err_finish(sk, transport);
> > }
> >
> > sctp_err_lookup() {
> > ...
> >         bh_lock_sock(sk);
> >
> >         /* If too many ICMPs get dropped on busy
> >          * servers this needs to be solved differently.
> >          */
> >         if (sock_owned_by_user(sk))            [A]
> >                 __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
> >
> >         *app = asoc;
> >         *tpp = transport;
> >         return sk;
> > ...
> > }
> >
> > Though that if() on [A] should be bailing out without returning
> > nothing. That's a bug. More like:
> >
> >         if (sock_owned_by_user(sk)) {
> >                 __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
> >                 goto out;
> >         }
> >
> 
> So why sctp_v4_err() is doing this test ?
> 
> if (!sock_owned_by_user(sk) && inet->recverr) {
> 
> It looks like socket can be owned by the user, and [A] check only
> increments an SNMP counter,
> that wont help to solve the tp->dst use after free.

Hah, missed that. Though the semantics on that counter still looks
confusing. It may be incremented when we actually handled the icmp.
The other icmp handling in there will postpone in case the socket is
locked by the user, and so will the timer callbacks too.

Will look more, thanks.

^ permalink raw reply

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Eric Dumazet @ 2017-10-17 17:20 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Xin Long, Eric Dumazet, Vlad Yasevich, Neil Horman, netdev,
	Wei Wang, linux-sctp
In-Reply-To: <20171017170132.GC5357@localhost.localdomain>

On Tue, Oct 17, 2017 at 10:01 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Oct 17, 2017 at 09:44:10AM -0700, Eric Dumazet wrote:
>> On Tue, Oct 17, 2017 at 9:28 AM, Marcelo Ricardo Leitner
>> <marcelo.leitner@gmail.com> wrote:
>> > On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
>> >> On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> >> > SCTP experts.
>> >> >
>> >> > syszkaller reported a few crashes in sctp_packet_config() with invalid
>> >> > access to a deleted dst.
>> >> >
>> >> > The rcu_read_lock() in sctp_packet_config() is suspect.
>> >> >
>> >> > It does not protect anything at the moment.
>> >> >
>> >> > If we expect tp->dst to be manipulated/changed by another cpu/thread,
>> >> > then we need proper rcu protection.
>> >> >
>> >> > Following patch to show what would be a minimal change (but obviously
>> >> > bigger changes are needed, like sctp_transport_pmtu_check() and
>> >> > sctp_transport_dst_check(), and proper sparse annotations)
>> >> will check all places accessing tp->dst in sctp.
>> >
>> > I checked some and sctp_transport_dst_check() should be fine because
>> > by then we are holding a reference on dst. Same goes to
>> > sctp_transport_pmtu_check().
>>
>> Really ?
>>
>
> Yes,
>
>> What about sctp_v4_err() -> sctp_icmp_redirect() -> sctp_transport_dst_check()
>>
>> It seems quite possible that the BH handler can access it, while
>> socket is owned by user.
>
> hidden here:
> sctp_v4_err() {
> ...
>         sk = sctp_err_lookup(net, AF_INET, skb, sctp_hdr(skb), &asoc,
>         &transport);
> ...
> out_unlock:
>         sctp_err_finish(sk, transport);
> }
>
> sctp_err_lookup() {
> ...
>         bh_lock_sock(sk);
>
>         /* If too many ICMPs get dropped on busy
>          * servers this needs to be solved differently.
>          */
>         if (sock_owned_by_user(sk))            [A]
>                 __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
>
>         *app = asoc;
>         *tpp = transport;
>         return sk;
> ...
> }
>
> Though that if() on [A] should be bailing out without returning
> nothing. That's a bug. More like:
>
>         if (sock_owned_by_user(sk)) {
>                 __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
>                 goto out;
>         }
>

So why sctp_v4_err() is doing this test ?

if (!sock_owned_by_user(sk) && inet->recverr) {

It looks like socket can be owned by the user, and [A] check only
increments an SNMP counter,
that wont help to solve the tp->dst use after free.



I

^ permalink raw reply

* Re: RFC(v2): Audit Kernel Container IDs
From: Steve Grubb @ 2017-10-17 17:15 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: James Bottomley, Simo Sorce, linux-audit, mszeredi, trondmy,
	jlayton, Linux API, Linux Containers, Linux Kernel, David Howells,
	Carlos O'Donell, cgroups, Eric W. Biederman, Andy Lutomirski,
	Linux Network Development, Linux FS Devel, Eric Paris, Al Viro
In-Reply-To: <eb96144d-4ab5-7f9f-de18-b296db35a00a@schaufler-ca.com>

On Tuesday, October 17, 2017 12:43:18 PM EDT Casey Schaufler wrote:
> > The idea is that processes spawned into a container would be labelled
> > by the container orchestration system.  It's unclear what should happen
> > to processes using nsenter after the fact, but policy for that should
> > be up to the orchestration system.
> 
> I'm fine with that. The user space policy can be anything y'all like.

I think there should be a login event.


> > The label will be used as a tag for audit information.
> 
> Deep breath ...
> 
> Which *is* a kernel security policy mechanism. Since the "label"
> is part of the audit information that the kernel is guaranteeing
> changing it would be covered by CAP_AUDIT_CONTROL. If the kernel
> does not use the "label" for any other purpose this is the only
> capability that makes sense for it.

I agree. The ability to set the container label grants the ability to evade 
rules or modify audit rules. CAP_AUDIT_CONTROL makes sense to me.


> > I think you were missing label inheritance above.
> > 
> > The security implications are that anything that can change the label
> > could also hide itself and its doings from the audit system and thus
> > would be used as a means to evade detection.

Yes. We have the same problem with loginuid. There are restrictions on who can 
change it once set. And then we made an immutable flag so that people that 
want a hard guarantee can get that.

-Steve

^ permalink raw reply

* [PATCH net-next] tun: relax check on eth_get_headlen() return value
From: Eric Dumazet @ 2017-10-17 17:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Petar Penkov

From: Eric Dumazet <edumazet@google.com>

syzkaller hit the WARN() in tun_get_user(), providing skb
with payload in fragments only, and nothing in skb->head

GRO layer is fine with this, so relax the check.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/tun.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 57e4c31fa84adc4d9af6ab69a87feac23a8b034e..c64ec19af9b73744270f5cdb922d0f0c1c8f4443 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1737,7 +1737,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		/* Exercise flow dissector code path. */
 		u32 headlen = eth_get_headlen(skb->data, skb_headlen(skb));
 
-		if (headlen > skb_headlen(skb) || headlen < ETH_HLEN) {
+		if (unlikely(headlen > skb_headlen(skb))) {
 			this_cpu_inc(tun->pcpu_stats->rx_dropped);
 			napi_free_frags(&tfile->napi);
 			mutex_unlock(&tfile->napi_mutex);

^ permalink raw reply related

* Do you need financial help?
From: Mr.Robert @ 2017-10-17 17:05 UTC (permalink / raw)
  To: Recipients

Do you need a loan to enhance your business?, loan to consolidate your debt,loan for personal use, loan for credit card, medical care loan,car loan,mortgage loan, student loan, loan for any purposes? e.t.c. get loan at 1% interest rate annually, hurry up now and fill out this below application details if interested: contact us at: iceblueinvestmentgroup01@gmail.com

APPLICATION DETAILS
Full Name---------------
Gender:___________________
Contact Address:____________
Country:______________________
State:__________________________
Amount Needed as Loan:___________
Loan Duration:____________________
Occupation:___________________ _____
Purpose for Loan:____________________
Phone:________________________ _______
Regard,

^ permalink raw reply

* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: Cong Wang @ 2017-10-17 17:05 UTC (permalink / raw)
  To: David Ahern; +Cc: Eric Dumazet, Linux Kernel Network Developers
In-Reply-To: <f0320a62-708b-6b7c-bf58-23b9e8df6cdc@gmail.com>

On Tue, Oct 17, 2017 at 8:32 AM, David Ahern <dsahern@gmail.com> wrote:
> On 10/16/17 9:45 PM, Eric Dumazet wrote:
>> IPV6 TCP uses sk->sk_v6_daddr and sk->->sk_v6_rcv_saddr
>
> I moved v2 to those.
>
>> So I would rather remove the need to fetch np = inet6_sk(sk) in the
>> first place, and look at sk->sk_family instead.
>
> I'll spin a v3 with that change.

Note, with CONFIG_IPV6=n, inet6_sk() is just NULL and compiler
could eliminate the if(NULL) branch, I doubt compiler could do same
for sk->sk_family.

Oh, BTW, I thought it is okay to use sk_v6_daddr in if (NULL)
case (aka CONFIG_IPV6=n), but a quick test shows compiler still
complains... so probably need #if IS_ENABLED(CONFIG_IPV6).

^ permalink raw reply

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Marcelo Ricardo Leitner @ 2017-10-17 17:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Xin Long, Eric Dumazet, Vlad Yasevich, Neil Horman, netdev,
	Wei Wang, linux-sctp
In-Reply-To: <CANn89iKMBKJyLpdnUEER=9ot89P8yr4F0DGJABh1O=DCixdR8g@mail.gmail.com>

On Tue, Oct 17, 2017 at 09:44:10AM -0700, Eric Dumazet wrote:
> On Tue, Oct 17, 2017 at 9:28 AM, Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> > On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
> >> On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >> > SCTP experts.
> >> >
> >> > syszkaller reported a few crashes in sctp_packet_config() with invalid
> >> > access to a deleted dst.
> >> >
> >> > The rcu_read_lock() in sctp_packet_config() is suspect.
> >> >
> >> > It does not protect anything at the moment.
> >> >
> >> > If we expect tp->dst to be manipulated/changed by another cpu/thread,
> >> > then we need proper rcu protection.
> >> >
> >> > Following patch to show what would be a minimal change (but obviously
> >> > bigger changes are needed, like sctp_transport_pmtu_check() and
> >> > sctp_transport_dst_check(), and proper sparse annotations)
> >> will check all places accessing tp->dst in sctp.
> >
> > I checked some and sctp_transport_dst_check() should be fine because
> > by then we are holding a reference on dst. Same goes to
> > sctp_transport_pmtu_check().
> 
> Really ?
> 

Yes,

> What about sctp_v4_err() -> sctp_icmp_redirect() -> sctp_transport_dst_check()
> 
> It seems quite possible that the BH handler can access it, while
> socket is owned by user.

hidden here:
sctp_v4_err() {
...
        sk = sctp_err_lookup(net, AF_INET, skb, sctp_hdr(skb), &asoc,
	&transport);
...
out_unlock:
        sctp_err_finish(sk, transport);
}

sctp_err_lookup() {
...
        bh_lock_sock(sk);

        /* If too many ICMPs get dropped on busy
         * servers this needs to be solved differently.
         */
        if (sock_owned_by_user(sk))            [A]
                __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);

        *app = asoc;
        *tpp = transport;
        return sk;
...
}

Though that if() on [A] should be bailing out without returning
nothing. That's a bug. More like:

        if (sock_owned_by_user(sk)) {
                __NET_INC_STATS(net, LINUX_MIB_LOCKDROPPEDICMPS);
		goto out;
	}

^ permalink raw reply

* Re: [patch net v3 2/4] net/sched: Use action array instead of action list as parameter
From: Cong Wang @ 2017-10-17 16:55 UTC (permalink / raw)
  To: Chris Mi
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Lucas Bates,
	Jiri Pirko, David Miller
In-Reply-To: <1508203218-17998-3-git-send-email-chrism@mellanox.com>

On Mon, Oct 16, 2017 at 6:20 PM, Chris Mi <chrism@mellanox.com> wrote:
> When destroying filters, actions should be destroyed first.
> The pointers of each action are saved in an array. TC doesn't
> use the array directly, but put all actions in a doubly linked
> list and use that list as parameter.
>
> There is no problem if each filter has its own actions. But if
> some filters share the same action, when these filters are
> destroyed, RCU callback fl_destroy_filter() may be called at the
> same time. That means the same action's 'struct list_head list'
> could be manipulated at the same time. It may point to an invalid
> address so that system will panic.

So if we remove these RCU callbacks (by adding a sychronize_rcu)
this is not a problem, right? Or is there any other races than RCU
callbacks??


>
> This patch uses the action array directly to fix this issue.
>
> Fixes commit in pre-git era.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")

This is wrong too. RCU callbacks were introduced very late.

^ permalink raw reply

* Re: [PATCH net-next] tcp: Enable TFO without a cookie on a per-socket basis
From: Christoph Paasch @ 2017-10-17 16:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Yuchung Cheng
In-Reply-To: <CANn89i+50uOGyyNKd8HMsPuZpmxFpsj28gHc7LAV_0BL9NpkEQ@mail.gmail.com>

On 17/10/17 - 04:00:01, Eric Dumazet wrote:
> On Mon, Oct 16, 2017 at 11:37 PM, Christoph Paasch <cpaasch@apple.com> wrote:
> > We already allow to enable TFO without a cookie by using the
> > fastopen-sysctl and setting it to TFO_SERVER_COOKIE_NOT_REQD (0x200).
> > This is safe to do in certain environments where we know that there
> > isn't a malicous host (aka., data-centers).
> >
> > A server however might be talking to both sides (public Internet and
> > data-center). So, this server would want to enable cookie-less TFO for
> > the connections that go to the data-center while enforcing cookies for
> > the traffic from the Internet.
> >
> > This patch exposes a socket-option to enable this (protected by
> > CAP_NET_ADMIN).
> 
> Have you thought instead of a route attribute ?

Another use-case for per-socket configuration is where the application-level
protocol already provides an authentication mechanism in the first flight of
data so that the cookie basically becomes redundant. In that case, it is
useful to configure it on a per-socket basis if other services are running
on this server as well.

I can of course add the route attribute in the v2, but I think the sockopt has
its use-case as well.

> CAP_NET_ADMIN restriction is not really practical IMO.

I'm fine with removing it.

I added it because an unpreviliged user could more easily mount an
amplification attack. But that is probably quite a stretch :)


Christoph

^ permalink raw reply

* [PATCH] udp: make some messages more descriptive
From: Matteo Croce @ 2017-10-17 16:48 UTC (permalink / raw)
  To: netdev

In the UDP code there are two leftover error messages with very few meaning.
Replace them with a more descriptive error message as some users
reported them as "strange network error".
---
 net/ipv4/udp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e45177ceb0ee..806b298a3bdd 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1061,7 +1061,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		/* ... which is an evident application bug. --ANK */
 		release_sock(sk);
 
-		net_dbg_ratelimited("cork app bug 2\n");
+		net_dbg_ratelimited("socket already corked\n");
 		err = -EINVAL;
 		goto out;
 	}
@@ -1144,7 +1144,7 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
 	if (unlikely(!up->pending)) {
 		release_sock(sk);
 
-		net_dbg_ratelimited("udp cork app bug 3\n");
+		net_dbg_ratelimited("cork failed\n");
 		return -EINVAL;
 	}
 
-- 
2.13.6

^ permalink raw reply related

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Eric Dumazet @ 2017-10-17 16:44 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Xin Long, Eric Dumazet, Vlad Yasevich, Neil Horman, netdev,
	Wei Wang, linux-sctp
In-Reply-To: <20171017162826.GB5357@localhost.localdomain>

On Tue, Oct 17, 2017 at 9:28 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
>> On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > SCTP experts.
>> >
>> > syszkaller reported a few crashes in sctp_packet_config() with invalid
>> > access to a deleted dst.
>> >
>> > The rcu_read_lock() in sctp_packet_config() is suspect.
>> >
>> > It does not protect anything at the moment.
>> >
>> > If we expect tp->dst to be manipulated/changed by another cpu/thread,
>> > then we need proper rcu protection.
>> >
>> > Following patch to show what would be a minimal change (but obviously
>> > bigger changes are needed, like sctp_transport_pmtu_check() and
>> > sctp_transport_dst_check(), and proper sparse annotations)
>> will check all places accessing tp->dst in sctp.
>
> I checked some and sctp_transport_dst_check() should be fine because
> by then we are holding a reference on dst. Same goes to
> sctp_transport_pmtu_check().

Really ?

What about sctp_v4_err() -> sctp_icmp_redirect() -> sctp_transport_dst_check()

It seems quite possible that the BH handler can access it, while
socket is owned by user.

^ permalink raw reply

* Re: RFC(v2): Audit Kernel Container IDs
From: Casey Schaufler @ 2017-10-17 16:43 UTC (permalink / raw)
  To: James Bottomley, Simo Sorce, Steve Grubb, linux-audit
  Cc: mszeredi, trondmy, jlayton, Linux API, Linux Containers,
	Linux Kernel, David Howells, Carlos O'Donell, cgroups,
	Eric W. Biederman, Andy Lutomirski, Linux Network Development,
	Linux FS Devel, Eric Paris, Al Viro
In-Reply-To: <1508255091.3129.27.camel@HansenPartnership.com>

On 10/17/2017 8:44 AM, James Bottomley wrote:
> On Tue, 2017-10-17 at 11:28 -0400, Simo Sorce wrote:
>>> Without a *kernel* policy on containerIDs you can't say what
>>> security policy is being exempted.
>> The policy has been basically stated earlier.
>>
>> A way to track a set of processes from a specific point in time
>> forward. The name used is "container id", but it could be anything.
>> This marker is mostly used by user space to track process hierarchies
>> without races, these processes can be very privileged, and must not
>> be allowed to change the marker themselves when granted the current
>> common capabilities.
>>
>> Is this a good enough description ? If not can you clarify your
>> expectations ?
> I think you mean you want to be able to apply a label to a process
> which is inherited across forks.

That would be PTAGS. I agree that such a general mechanism
could be very useful for a variety of purposes, not just
containers. I do not agree that a single integer (e.g. a
containerID) warrants more than trivial mechanism.

> The label should only be susceptible
> to modification by something possessing a capability (which one TBD).

I think that the reason we're going to have crying and gnashing
of teeth is that whatever capability is used. There will always be
an issue of the capability granted being less specific than the
application security model would like.

And no, we're not going down the 330 capabilities road. It's been
done in the UNIX world. Application security models hate that
just as much as they hate the coarser granularity.

> The idea is that processes spawned into a container would be labelled
> by the container orchestration system.  It's unclear what should happen
> to processes using nsenter after the fact, but policy for that should
> be up to the orchestration system.

I'm fine with that. The user space policy can be anything y'all like.

> The label will be used as a tag for audit information.

Deep breath ...

Which *is* a kernel security policy mechanism. Since the "label"
is part of the audit information that the kernel is guaranteeing
changing it would be covered by CAP_AUDIT_CONTROL. If the kernel
does not use the "label" for any other purpose this is the only
capability that makes sense for it.

> I think you were missing label inheritance above.
>
> The security implications are that anything that can change the label
> could also hide itself and its doings from the audit system and thus
> would be used as a means to evade detection.  

Yes. This is a consequence of the capability granularity. There is
no way we can make the capability granularity sufficiently fine to
prevent this. No one wants the 330 capabilities that Data General
had in their secure UNIX system. 

> I actually think this
> means the label should be write once (once you've set it, you can't
> change it) and orchestration systems should begin as unlabelled
> processes allowing them to do arbitrary forks.
>
> For nested containers, I actually think the label should be
> hierarchical, so you can add a label for the new nested container but
> it still also contains its parents label as well.

You can't support this reasonably with a single containerID.
You want PTAGS for this. I know that there is resistance to
requiring anything beyond what's in the base kernel (and for
good reasons) for containers. Especially something that is
pending future work. But let's not jam something into the base
kernel that isn't really going to address the issue.

> James

^ permalink raw reply

* Re: [PATCH 0/4] make function arg and structures as const
From: Jeff Layton @ 2017-10-17 16:40 UTC (permalink / raw)
  To: Bhumika Goyal, julia.lawall, trond.myklebust, anna.schumaker,
	bfields, davem, linux-nfs, linux-kernel, netdev
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

On Tue, 2017-10-17 at 18:14 +0200, Bhumika Goyal wrote:
> Make the function argument as const. After thing change, make
> the cache_detail structures as const.
> 
> Bhumika Goyal (4):
>   sunrpc: make the function arg as const
>   NFS: make cache_detail structures const
>   NFSD: make cache_detail structures const
>   SUNRPC: make cache_detail structures const
> 
>  fs/nfs/dns_resolve.c              | 2 +-
>  fs/nfsd/export.c                  | 4 ++--
>  fs/nfsd/nfs4idmap.c               | 4 ++--
>  include/linux/sunrpc/cache.h      | 2 +-
>  net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
>  net/sunrpc/cache.c                | 2 +-
>  net/sunrpc/svcauth_unix.c         | 4 ++--
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 

Looks pretty straightforward. You can add this to the set:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

^ permalink raw reply

* Re: [PATCH net 3/3] bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
From: John Fastabend @ 2017-10-17 16:29 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: tj, ast, mark.rutland, richard, sp3485, netdev, linux-kernel
In-Reply-To: <a11e92b8ba4f5270437caa629307c300bca37976.1508251210.git.daniel@iogearbox.net>

On 10/17/2017 07:55 AM, Daniel Borkmann wrote:
> PCPU_MIN_UNIT_SIZE is an implementation detail of the percpu
> allocator. Given we support __GFP_NOWARN now, lets just let
> the allocation request fail naturally instead. The two call
> sites from BPF mistakenly assumed __GFP_NOWARN would work, so
> no changes needed to their actual __alloc_percpu_gfp() calls
> which use the flag already.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---

Nice cleanup. Thanks!

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply

* Re: [PATCH net 2/3] bpf: fix splat for illegal devmap percpu allocation
From: John Fastabend @ 2017-10-17 16:28 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: tj, ast, mark.rutland, richard, sp3485, netdev, linux-kernel
In-Reply-To: <dd091b1fec763d74185a156ecd2f972810bdd1ce.1508251210.git.daniel@iogearbox.net>

On 10/17/2017 07:55 AM, Daniel Borkmann wrote:
> It was reported that syzkaller was able to trigger a splat on
> devmap percpu allocation due to illegal/unsupported allocation
> request size passed to __alloc_percpu():
> 
>   [   70.094249] illegal size (32776) or align (8) for percpu allocation
>   [   70.094256] ------------[ cut here ]------------
>   [   70.094259] WARNING: CPU: 3 PID: 3451 at mm/percpu.c:1365 pcpu_alloc+0x96/0x630
>   [...]
>   [   70.094325] Call Trace:
>   [   70.094328]  __alloc_percpu_gfp+0x12/0x20
>   [   70.094330]  dev_map_alloc+0x134/0x1e0
>   [   70.094331]  SyS_bpf+0x9bc/0x1610
>   [   70.094333]  ? selinux_task_setrlimit+0x5a/0x60
>   [   70.094334]  ? security_task_setrlimit+0x43/0x60
>   [   70.094336]  entry_SYSCALL_64_fastpath+0x1a/0xa5
> 
> This was due to too large max_entries for the map such that we
> surpassed the upper limit of PCPU_MIN_UNIT_SIZE. It's fine to
> fail naturally here, so switch to __alloc_percpu_gfp() and pass
> __GFP_NOWARN instead.
> 
> Fixes: 11393cc9b9be ("xdp: Add batching support to redirect map")
> Reported-by: Mark Rutland <mark.rutland@arm.com>
> Reported-by: Shankara Pailoor <sp3485@columbia.edu>
> Reported-by: Richard Weinberger <richard@nod.at>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: John Fastabend <john.fastabend@gmail.com>
> ---

Thanks!

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Marcelo Ricardo Leitner @ 2017-10-17 16:28 UTC (permalink / raw)
  To: Xin Long
  Cc: Eric Dumazet, Vlad Yasevich, Neil Horman, netdev, Wei Wang,
	Eric Dumazet, linux-sctp
In-Reply-To: <CADvbK_dRBaOLzrXWS2ndHu3sz9LB87rJsyGog2BjishapiMhxw@mail.gmail.com>

On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
> On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > SCTP experts.
> >
> > syszkaller reported a few crashes in sctp_packet_config() with invalid
> > access to a deleted dst.
> >
> > The rcu_read_lock() in sctp_packet_config() is suspect.
> >
> > It does not protect anything at the moment.
> >
> > If we expect tp->dst to be manipulated/changed by another cpu/thread,
> > then we need proper rcu protection.
> >
> > Following patch to show what would be a minimal change (but obviously
> > bigger changes are needed, like sctp_transport_pmtu_check() and
> > sctp_transport_dst_check(), and proper sparse annotations)
> will check all places accessing tp->dst in sctp.

I checked some and sctp_transport_dst_check() should be fine because
by then we are holding a reference on dst. Same goes to
sctp_transport_pmtu_check().  It's not possible that these would trip
on the update going on on sctp_packet_config() because the socket is
locked. We may not need (much) more than the example patch, I think.

A more thorough check is certainly welcomed, indeed.

  Marcelo

^ permalink raw reply

* Re: [PATCH 44/58] net: ethernet: sun: Convert timers to use timer_setup()
From: Shannon Nelson @ 2017-10-17 16:27 UTC (permalink / raw)
  To: Kees Cook, David S. Miller
  Cc: Philippe Reynes, Jarod Wilson, Rob Herring, chris hyser,
	Tushar Dave, Tobias Klauser, netdev, Thomas Gleixner,
	linux-kernel
In-Reply-To: <1508200182-104605-45-git-send-email-keescook@chromium.org>

On 10/16/2017 5:29 PM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Cc: Jarod Wilson <jarod@redhat.com>
> Cc: Shannon Nelson <shannon.nelson@oracle.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: chris hyser <chris.hyser@oracle.com>
> Cc: Tushar Dave <tushar.n.dave@oracle.com>
> Cc: Tobias Klauser <tklauser@distanz.ch>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Acked-by: Shannon Nelson <shannon.nelson@oracle.com>

> ---
>   drivers/net/ethernet/sun/cassini.c        |  7 ++++---
>   drivers/net/ethernet/sun/ldmvsw.c         |  3 +--
>   drivers/net/ethernet/sun/niu.c            | 10 ++++------
>   drivers/net/ethernet/sun/sunbmac.c        | 10 ++++------
>   drivers/net/ethernet/sun/sungem.c         |  6 +++---
>   drivers/net/ethernet/sun/sunhme.c         | 10 ++++------
>   drivers/net/ethernet/sun/sunvnet.c        |  3 +--
>   drivers/net/ethernet/sun/sunvnet_common.c |  4 ++--
>   drivers/net/ethernet/sun/sunvnet_common.h |  2 +-
>   9 files changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
> index a74d78f64af9..113bd57e2ea0 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -4079,9 +4079,9 @@ static void cas_reset_task(struct work_struct *work)
>   #endif
>   }
>   
> -static void cas_link_timer(unsigned long data)
> +static void cas_link_timer(struct timer_list *t)
>   {
> -	struct cas *cp = (struct cas *) data;
> +	struct cas *cp = from_timer(cp, t, link_timer);
>   	int mask, pending = 0, reset = 0;
>   	unsigned long flags;
>   
> @@ -5039,7 +5039,8 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	spin_lock_init(&cp->stat_lock[N_TX_RINGS]);
>   	mutex_init(&cp->pm_mutex);
>   
> -	setup_timer(&cp->link_timer, cas_link_timer, (unsigned long)cp);
> +	timer_setup(&cp->link_timer, cas_link_timer, 0);
> +
>   #if 1
>   	/* Just in case the implementation of atomic operations
>   	 * change so that an explicit initialization is necessary.
> diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
> index 5feeaa9f0a9e..5ea037672e6f 100644
> --- a/drivers/net/ethernet/sun/ldmvsw.c
> +++ b/drivers/net/ethernet/sun/ldmvsw.c
> @@ -363,8 +363,7 @@ static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>   	list_add_rcu(&port->list, &vp->port_list);
>   	spin_unlock_irqrestore(&vp->lock, flags);
>   
> -	setup_timer(&port->clean_timer, sunvnet_clean_timer_expire_common,
> -		    (unsigned long)port);
> +	timer_setup(&port->clean_timer, sunvnet_clean_timer_expire_common, 0);
>   
>   	err = register_netdev(dev);
>   	if (err) {
> diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
> index bde19b307d0d..ab502ee35fb2 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -2221,9 +2221,9 @@ static int niu_link_status(struct niu *np, int *link_up_p)
>   	return err;
>   }
>   
> -static void niu_timer(unsigned long __opaque)
> +static void niu_timer(struct timer_list *t)
>   {
> -	struct niu *np = (struct niu *) __opaque;
> +	struct niu *np = from_timer(np, t, timer);
>   	unsigned long off;
>   	int err, link_up;
>   
> @@ -6123,7 +6123,7 @@ static int niu_open(struct net_device *dev)
>   
>   	err = niu_init_hw(np);
>   	if (!err) {
> -		setup_timer(&np->timer, niu_timer, (unsigned long)np);
> +		timer_setup(&np->timer, niu_timer, 0);
>   		np->timer.expires = jiffies + HZ;
>   
>   		err = niu_enable_interrupts(np, 1);
> @@ -6773,10 +6773,8 @@ static int niu_change_mtu(struct net_device *dev, int new_mtu)
>   
>   	err = niu_init_hw(np);
>   	if (!err) {
> -		init_timer(&np->timer);
> +		timer_setup(&np->timer, niu_timer, 0);
>   		np->timer.expires = jiffies + HZ;
> -		np->timer.data = (unsigned long) np;
> -		np->timer.function = niu_timer;
>   
>   		err = niu_enable_interrupts(np, 1);
>   		if (err)
> diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
> index 3189722110c2..0b1f41f6bceb 100644
> --- a/drivers/net/ethernet/sun/sunbmac.c
> +++ b/drivers/net/ethernet/sun/sunbmac.c
> @@ -523,9 +523,9 @@ static int try_next_permutation(struct bigmac *bp, void __iomem *tregs)
>   	return -1;
>   }
>   
> -static void bigmac_timer(unsigned long data)
> +static void bigmac_timer(struct timer_list *t)
>   {
> -	struct bigmac *bp = (struct bigmac *) data;
> +	struct bigmac *bp = from_timer(bp, t, bigmac_timer);
>   	void __iomem *tregs = bp->tregs;
>   	int restart_timer = 0;
>   
> @@ -613,8 +613,6 @@ static void bigmac_begin_auto_negotiation(struct bigmac *bp)
>   	bp->timer_state = ltrywait;
>   	bp->timer_ticks = 0;
>   	bp->bigmac_timer.expires = jiffies + (12 * HZ) / 10;
> -	bp->bigmac_timer.data = (unsigned long) bp;
> -	bp->bigmac_timer.function = bigmac_timer;
>   	add_timer(&bp->bigmac_timer);
>   }
>   
> @@ -921,7 +919,7 @@ static int bigmac_open(struct net_device *dev)
>   		printk(KERN_ERR "BIGMAC: Can't order irq %d to go.\n", dev->irq);
>   		return ret;
>   	}
> -	init_timer(&bp->bigmac_timer);
> +	timer_setup(&bp->bigmac_timer, bigmac_timer, 0);
>   	ret = bigmac_init_hw(bp, 0);
>   	if (ret)
>   		free_irq(dev->irq, bp);
> @@ -1172,7 +1170,7 @@ static int bigmac_ether_init(struct platform_device *op,
>   					      "board-version", 1);
>   
>   	/* Init auto-negotiation timer state. */
> -	init_timer(&bp->bigmac_timer);
> +	timer_setup(&bp->bigmac_timer, bigmac_timer, 0);
>   	bp->timer_state = asleep;
>   	bp->timer_ticks = 0;
>   
> diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
> index b75ab8f44968..a7afcee3c5ae 100644
> --- a/drivers/net/ethernet/sun/sungem.c
> +++ b/drivers/net/ethernet/sun/sungem.c
> @@ -1496,9 +1496,9 @@ static int gem_mdio_link_not_up(struct gem *gp)
>   	}
>   }
>   
> -static void gem_link_timer(unsigned long data)
> +static void gem_link_timer(struct timer_list *t)
>   {
> -	struct gem *gp = (struct gem *) data;
> +	struct gem *gp = from_timer(gp, t, link_timer);
>   	struct net_device *dev = gp->dev;
>   	int restart_aneg = 0;
>   
> @@ -2910,7 +2910,7 @@ static int gem_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>   
>   	gp->msg_enable = DEFAULT_MSG;
>   
> -	setup_timer(&gp->link_timer, gem_link_timer, (unsigned long)gp);
> +	timer_setup(&gp->link_timer, gem_link_timer, 0);
>   
>   	INIT_WORK(&gp->reset_task, gem_reset_task);
>   
> diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
> index 9e983e1d8249..0431f1e5f511 100644
> --- a/drivers/net/ethernet/sun/sunhme.c
> +++ b/drivers/net/ethernet/sun/sunhme.c
> @@ -685,9 +685,9 @@ static int is_lucent_phy(struct happy_meal *hp)
>   	return ret;
>   }
>   
> -static void happy_meal_timer(unsigned long data)
> +static void happy_meal_timer(struct timer_list *t)
>   {
> -	struct happy_meal *hp = (struct happy_meal *) data;
> +	struct happy_meal *hp = from_timer(hp, t, happy_timer);
>   	void __iomem *tregs = hp->tcvregs;
>   	int restart_timer = 0;
>   
> @@ -1413,8 +1413,6 @@ happy_meal_begin_auto_negotiation(struct happy_meal *hp,
>   
>   	hp->timer_ticks = 0;
>   	hp->happy_timer.expires = jiffies + (12 * HZ)/10;  /* 1.2 sec. */
> -	hp->happy_timer.data = (unsigned long) hp;
> -	hp->happy_timer.function = happy_meal_timer;
>   	add_timer(&hp->happy_timer);
>   }
>   
> @@ -2819,7 +2817,7 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe)
>   	hp->timer_state = asleep;
>   	hp->timer_ticks = 0;
>   
> -	init_timer(&hp->happy_timer);
> +	timer_setup(&hp->happy_timer, happy_meal_timer, 0);
>   
>   	hp->dev = dev;
>   	dev->netdev_ops = &hme_netdev_ops;
> @@ -3133,7 +3131,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev,
>   	hp->timer_state = asleep;
>   	hp->timer_ticks = 0;
>   
> -	init_timer(&hp->happy_timer);
> +	timer_setup(&hp->happy_timer, happy_meal_timer, 0);
>   
>   	hp->irq = pdev->irq;
>   	hp->dev = dev;
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index 0b95105f7060..27fb22638885 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -492,8 +492,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>   	pr_info("%s: PORT ( remote-mac %pM%s )\n",
>   		vp->dev->name, port->raddr, switch_port ? " switch-port" : "");
>   
> -	setup_timer(&port->clean_timer, sunvnet_clean_timer_expire_common,
> -		    (unsigned long)port);
> +	timer_setup(&port->clean_timer, sunvnet_clean_timer_expire_common, 0);
>   
>   	napi_enable(&port->napi);
>   	vio_port_up(&port->vio);
> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
> index ecf456c7b6d1..8aa3ce46bb81 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.c
> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
> @@ -1040,9 +1040,9 @@ static inline void vnet_free_skbs(struct sk_buff *skb)
>   	}
>   }
>   
> -void sunvnet_clean_timer_expire_common(unsigned long port0)
> +void sunvnet_clean_timer_expire_common(struct timer_list *t)
>   {
> -	struct vnet_port *port = (struct vnet_port *)port0;
> +	struct vnet_port *port = from_timer(port, t, clean_timer);
>   	struct sk_buff *freeskbs;
>   	unsigned pending;
>   
> diff --git a/drivers/net/ethernet/sun/sunvnet_common.h b/drivers/net/ethernet/sun/sunvnet_common.h
> index b20d6fa7ef25..656673c31066 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.h
> +++ b/drivers/net/ethernet/sun/sunvnet_common.h
> @@ -129,7 +129,7 @@ struct vnet {
>   	((__port)->vsw ? (__port)->dev : (__port)->vp->dev)
>   
>   /* Common funcs */
> -void sunvnet_clean_timer_expire_common(unsigned long port0);
> +void sunvnet_clean_timer_expire_common(struct timer_list *t);
>   int sunvnet_open_common(struct net_device *dev);
>   int sunvnet_close_common(struct net_device *dev);
>   void sunvnet_set_rx_mode_common(struct net_device *dev, struct vnet *vp);
> 

^ permalink raw reply

* Re: [PATCH net] sctp: do not peel off an assoc from one netns to another one
From: Marcelo Ricardo Leitner @ 2017-10-17 16:24 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, chunwang, syzkaller
In-Reply-To: <e637d68ce6f4f94dce8cb30c647e672ebb1f0b7b.1508253970.git.lucien.xin@gmail.com>

On Tue, Oct 17, 2017 at 11:26:10PM +0800, Xin Long wrote:
> Now when peeling off an association to the sock in another netns, all
> transports in this assoc are not to be rehashed and keep use the old
> key in hashtable.
> 
> As a transport uses sk->net as the hash key to insert into hashtable,
> it would miss removing these transports from hashtable due to the new
> netns when closing the sock and all transports are being freeed, then
> later an use-after-free issue could be caused when looking up an asoc
> and dereferencing those transports.
> 
> This is a very old issue since very beginning, ChunYu found it with
> syzkaller fuzz testing with this series:
> 
>   socket$inet6_sctp()
>   bind$inet6()
>   sendto$inet6()
>   unshare(0x40000000)
>   getsockopt$inet_sctp6_SCTP_GET_ASSOC_ID_LIST()
>   getsockopt$inet_sctp6_SCTP_SOCKOPT_PEELOFF()
> 
> This patch is to block this call when peeling one assoc off from one
> netns to another one, so that the netns of all transport would not
> go out-sync with the key in hashtable.
> 
> Note that this patch didn't fix it by rehashing transports, as it's
> difficult to handle the situation when the tuple is already in use
> in the new netns. Besides, no one would like to peel off one assoc
> to another netns, considering ipaddrs, ifaces, etc. are usually
> different.
> 
> Reported-by: ChunYu Wang <chunwang@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/socket.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index d4730ad..17841ab 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4906,6 +4906,10 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
>  	struct socket *sock;
>  	int err = 0;
>  
> +	/* Do not peel off from one netns to another one. */
> +	if (!net_eq(current->nsproxy->net_ns, sock_net(sk)))
> +		return -EINVAL;
> +
>  	if (!asoc)
>  		return -EINVAL;
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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

* [PATCH 2/4] NFS: make cache_detail structure const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

Make it const as it is only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 fs/nfs/dns_resolve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index d25f10f..477934a 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -353,7 +353,7 @@ ssize_t nfs_dns_resolve_name(struct net *net, char *name,
 	return ret;
 }
 
-static struct cache_detail nfs_dns_resolve_template = {
+static const struct cache_detail nfs_dns_resolve_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= NFS_DNS_HASHTBL_SIZE,
 	.name		= "dns_resolve",
-- 
1.9.1

^ permalink raw reply related

* [PATCH 4/4] SUNRPC: make cache_detail structures const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

Make these const as they are only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
 net/sunrpc/svcauth_unix.c         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 7b1ee5a..e242cb4 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -264,7 +264,7 @@ static int rsi_parse(struct cache_detail *cd,
 	return status;
 }
 
-static struct cache_detail rsi_cache_template = {
+static const struct cache_detail rsi_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= RSI_HASHMAX,
 	.name           = "auth.rpcsec.init",
@@ -524,7 +524,7 @@ static int rsc_parse(struct cache_detail *cd,
 	return status;
 }
 
-static struct cache_detail rsc_cache_template = {
+static const struct cache_detail rsc_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= RSC_HASHMAX,
 	.name		= "auth.rpcsec.context",
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index f81eaa8..740b67d 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -569,7 +569,7 @@ static int unix_gid_show(struct seq_file *m,
 	return 0;
 }
 
-static struct cache_detail unix_gid_cache_template = {
+static const struct cache_detail unix_gid_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= GID_HASHMAX,
 	.name		= "auth.unix.gid",
@@ -862,7 +862,7 @@ struct auth_ops svcauth_unix = {
 	.set_client	= svcauth_unix_set_client,
 };
 
-static struct cache_detail ip_map_cache_template = {
+static const struct cache_detail ip_map_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= IP_HASHMAX,
 	.name		= "auth.unix.ip",
-- 
1.9.1

^ permalink raw reply related

* [PATCH 3/4] NFSD: make cache_detail structures const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall-L2FTfq7BK8M, trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Make these const as they are only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/nfsd/export.c    | 4 ++--
 fs/nfsd/nfs4idmap.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 3bc08c3..06bb39c 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -231,7 +231,7 @@ static struct cache_head *expkey_alloc(void)
 		return NULL;
 }
 
-static struct cache_detail svc_expkey_cache_template = {
+static const struct cache_detail svc_expkey_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= EXPKEY_HASHMAX,
 	.name		= "nfsd.fh",
@@ -747,7 +747,7 @@ static struct cache_head *svc_export_alloc(void)
 		return NULL;
 }
 
-static struct cache_detail svc_export_cache_template = {
+static const struct cache_detail svc_export_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= EXPORT_HASHMAX,
 	.name		= "nfsd.export",
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 6b9b6cc..a5bb765 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -178,7 +178,7 @@ struct ent {
 static struct ent *idtoname_update(struct cache_detail *, struct ent *,
 				   struct ent *);
 
-static struct cache_detail idtoname_cache_template = {
+static const struct cache_detail idtoname_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.idtoname",
@@ -341,7 +341,7 @@ static struct ent *nametoid_update(struct cache_detail *, struct ent *,
 				   struct ent *);
 static int         nametoid_parse(struct cache_detail *, char *, int);
 
-static struct cache_detail nametoid_cache_template = {
+static const struct cache_detail nametoid_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.nametoid",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 1/4] sunrpc: make the function arg as const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall-L2FTfq7BK8M, trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Make the struct cache_detail *tmpl argument of the function
cache_create_net as const as it is only getting passed to kmemup having
the argument as const void *.
Add const to the prototype too.

Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 include/linux/sunrpc/cache.h | 2 +-
 net/sunrpc/cache.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 270bad0..40d2822 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -213,7 +213,7 @@ extern int cache_check(struct cache_detail *detail,
 extern int cache_register_net(struct cache_detail *cd, struct net *net);
 extern void cache_unregister_net(struct cache_detail *cd, struct net *net);
 
-extern struct cache_detail *cache_create_net(struct cache_detail *tmpl, struct net *net);
+extern struct cache_detail *cache_create_net(const struct cache_detail *tmpl, struct net *net);
 extern void cache_destroy_net(struct cache_detail *cd, struct net *net);
 
 extern void sunrpc_init_cache_detail(struct cache_detail *cd);
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 79d55d9..e689438 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1674,7 +1674,7 @@ void cache_unregister_net(struct cache_detail *cd, struct net *net)
 }
 EXPORT_SYMBOL_GPL(cache_unregister_net);
 
-struct cache_detail *cache_create_net(struct cache_detail *tmpl, struct net *net)
+struct cache_detail *cache_create_net(const struct cache_detail *tmpl, struct net *net)
 {
 	struct cache_detail *cd;
 	int i;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 0/4] make function arg and structures as const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal

Make the function argument as const. After thing change, make
the cache_detail structures as const.

Bhumika Goyal (4):
  sunrpc: make the function arg as const
  NFS: make cache_detail structures const
  NFSD: make cache_detail structures const
  SUNRPC: make cache_detail structures const

 fs/nfs/dns_resolve.c              | 2 +-
 fs/nfsd/export.c                  | 4 ++--
 fs/nfsd/nfs4idmap.c               | 4 ++--
 include/linux/sunrpc/cache.h      | 2 +-
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
 net/sunrpc/cache.c                | 2 +-
 net/sunrpc/svcauth_unix.c         | 4 ++--
 7 files changed, 11 insertions(+), 11 deletions(-)

-- 
1.9.1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox