| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170407080433.GC16413@dhcp22.suse.cz>
Date: Fri, 7 Apr 2017 10:04:33 +0200
From: Michal Hocko <mhocko@...nel.org>
To: NeilBrown <neilb@...e.com>
Cc: Ingo Molnar <mingo@...hat.com>,
Peter Zijlstra <peterz@...radead.org>,
Mel Gorman <mgorman@...e.de>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Rename tsk_restore_flags() to current_restore_flags()
On Fri 07-04-17 10:03:26, NeilBrown wrote:
>
> It is not safe for one thread to modify the ->flags
> of another thread as there is no locking that can protect
> the update.
> So tsk_restore_flags(), which takes a task pointer and modifies
> the flags, is an invitation to do the wrong thing.
>
> All current users pass "current" as the task, so no developers have
> accepted that invitation. It would be best to ensure it remains
> that way.
>
> So rename tsk_restore_flags() to current_restore_flags() and don't
> pass in a task_struct pointer. Always operate on current->flags.
yes this makes a lot of sense to me.
It would be great if somebody cleaned up setting of ->flags to be always
set on current as well. This seems to be the case but there are places
where we do
struct task_struct *tsk = current;
[...]
tsk->flags |= PF_$FOO
> Signed-off-by: NeilBrown <neilb@...e.com>
Acked-by: Michal Hocko <mhocko@...e.com>
> ---
>
> get_maintainer.pl suggested 35 recipients for this patch.
> I decided to trim that a bit ....
>
> Can it just go through the tip tree?? It should be completely
> uncontroversial.
>
> Thanks,
> NeilBrown
>
> drivers/block/nbd.c | 2 +-
> drivers/scsi/iscsi_tcp.c | 2 +-
> fs/nfsd/vfs.c | 2 +-
> include/linux/sched.h | 6 +++---
> kernel/softirq.c | 2 +-
> net/core/dev.c | 2 +-
> net/core/sock.c | 2 +-
> 7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index 7e4287bc19e5..04ec921727b7 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -239,7 +239,7 @@ static int sock_xmit(struct nbd_device *nbd, int index, int send,
> }
> } while (msg_data_left(&msg));
>
> - tsk_restore_flags(current, pflags, PF_MEMALLOC);
> + current_restore_flags(pflags, PF_MEMALLOC);
>
> return result;
> }
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index 4228aba1f654..bbea8eac9abb 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -387,7 +387,7 @@ static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task)
> rc = 0;
> }
>
> - tsk_restore_flags(current, pflags, PF_MEMALLOC);
> + current_restore_flags(pflags, PF_MEMALLOC);
> return rc;
> }
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 19d50f600e8d..9aaf6ca77569 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1004,7 +1004,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
> else
> err = nfserrno(host_err);
> if (test_bit(RQ_LOCAL, &rqstp->rq_flags))
> - tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
> + current_restore_flags(pflags, PF_LESS_THROTTLE);
> return err;
> }
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d67eee84fd43..0978fb74e45a 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1286,10 +1286,10 @@ TASK_PFA_TEST(LMK_WAITING, lmk_waiting)
> TASK_PFA_SET(LMK_WAITING, lmk_waiting)
>
> static inline void
> -tsk_restore_flags(struct task_struct *task, unsigned long orig_flags, unsigned long flags)
> +current_restore_flags(unsigned long orig_flags, unsigned long flags)
> {
> - task->flags &= ~flags;
> - task->flags |= orig_flags & flags;
> + current->flags &= ~flags;
> + current->flags |= orig_flags & flags;
> }
>
> extern int cpuset_cpumask_can_shrink(const struct cpumask *cur, const struct cpumask *trial);
> diff --git a/kernel/softirq.c b/kernel/softirq.c
> index 744fa611cae0..4e09821f9d9e 100644
> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -309,7 +309,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
> account_irq_exit_time(current);
> __local_bh_enable(SOFTIRQ_OFFSET);
> WARN_ON_ONCE(in_interrupt());
> - tsk_restore_flags(current, old_flags, PF_MEMALLOC);
> + current_restore_flags(old_flags, PF_MEMALLOC);
> }
>
> asmlinkage __visible void do_softirq(void)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7869ae3837ca..e8a366387a99 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4240,7 +4240,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
> */
> current->flags |= PF_MEMALLOC;
> ret = __netif_receive_skb_core(skb, true);
> - tsk_restore_flags(current, pflags, PF_MEMALLOC);
> + current_restore_flags(pflags, PF_MEMALLOC);
> } else
> ret = __netif_receive_skb_core(skb, false);
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index a96d5f7a5734..20fb01b01ddb 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -325,7 +325,7 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
>
> current->flags |= PF_MEMALLOC;
> ret = sk->sk_backlog_rcv(sk, skb);
> - tsk_restore_flags(current, pflags, PF_MEMALLOC);
> + current_restore_flags(pflags, PF_MEMALLOC);
>
> return ret;
> }
> --
> 2.12.2
>
--
Michal Hocko
SUSE Labs
Powered by blists - more mailing lists