| 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: <907441886ba11255ec6be480c1fd90a1f23f959c.1602209970.git.skhan@linuxfoundation.org>
Date: Fri, 9 Oct 2020 09:56:02 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: gregkh@...uxfoundation.org, arve@...roid.com, tkjos@...roid.com,
maco@...roid.com, joel@...lfernandes.org, christian@...uner.io,
hridya@...gle.com, surenb@...gle.com, keescook@...omium.org
Cc: Shuah Khan <skhan@...uxfoundation.org>, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org,
Christian Brauner <christian.brauner@...ntu.com>
Subject: [PATCH v3 07/11] drivers/android/binder: convert stats, transaction_log to counter_atomic32
counter_atomic* is introduced to be used when a variable is used as
a simple counter and doesn't guard object lifetimes. This clearly
differentiates atomic_t usages that guard object lifetimes.
counter_atomic* variables wrap around to INT_MIN when it overflows and
should not be used to guard resource lifetimes, device usage and open
counts that control state changes, and pm states.
stats tracks per-process binder statistics. Unsure if there is a chance
of this overflowing, other than stats getting reset to 0. Convert it to
use counter_atomic.
binder_transaction_log:cur is used to keep track of the current log entry
location. Overflow is handled in the code. Since it is used as a
counter, convert it to use counter_atomic32.
This conversion doesn't change the overflow wrap around behavior.
Reviewed-by: Joel Fernandes (Google) <joel@...lfernandes.org>
Reviewed-by: Kees Cook <keescook@...omium.org>
Reviewed-by: Christian Brauner <christian.brauner@...ntu.com>
Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>
---
drivers/android/binder.c | 41 ++++++++++++++++---------------
drivers/android/binder_internal.h | 3 ++-
2 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index f936530a19b0..52175cd6a62b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -66,6 +66,7 @@
#include <linux/syscalls.h>
#include <linux/task_work.h>
#include <linux/sizes.h>
+#include <linux/counters.h>
#include <uapi/linux/android/binder.h>
#include <uapi/linux/android/binderfs.h>
@@ -172,22 +173,22 @@ enum binder_stat_types {
};
struct binder_stats {
- atomic_t br[_IOC_NR(BR_FAILED_REPLY) + 1];
- atomic_t bc[_IOC_NR(BC_REPLY_SG) + 1];
- atomic_t obj_created[BINDER_STAT_COUNT];
- atomic_t obj_deleted[BINDER_STAT_COUNT];
+ struct counter_atomic32 br[_IOC_NR(BR_FAILED_REPLY) + 1];
+ struct counter_atomic32 bc[_IOC_NR(BC_REPLY_SG) + 1];
+ struct counter_atomic32 obj_created[BINDER_STAT_COUNT];
+ struct counter_atomic32 obj_deleted[BINDER_STAT_COUNT];
};
static struct binder_stats binder_stats;
static inline void binder_stats_deleted(enum binder_stat_types type)
{
- atomic_inc(&binder_stats.obj_deleted[type]);
+ counter_atomic32_inc(&binder_stats.obj_deleted[type]);
}
static inline void binder_stats_created(enum binder_stat_types type)
{
- atomic_inc(&binder_stats.obj_created[type]);
+ counter_atomic32_inc(&binder_stats.obj_created[type]);
}
struct binder_transaction_log binder_transaction_log;
@@ -197,7 +198,7 @@ static struct binder_transaction_log_entry *binder_transaction_log_add(
struct binder_transaction_log *log)
{
struct binder_transaction_log_entry *e;
- unsigned int cur = atomic_inc_return(&log->cur);
+ unsigned int cur = counter_atomic32_inc_return(&log->cur);
if (cur >= ARRAY_SIZE(log->entry))
log->full = true;
@@ -3615,9 +3616,9 @@ static int binder_thread_write(struct binder_proc *proc,
ptr += sizeof(uint32_t);
trace_binder_command(cmd);
if (_IOC_NR(cmd) < ARRAY_SIZE(binder_stats.bc)) {
- atomic_inc(&binder_stats.bc[_IOC_NR(cmd)]);
- atomic_inc(&proc->stats.bc[_IOC_NR(cmd)]);
- atomic_inc(&thread->stats.bc[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&binder_stats.bc[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&proc->stats.bc[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&thread->stats.bc[_IOC_NR(cmd)]);
}
switch (cmd) {
case BC_INCREFS:
@@ -4047,9 +4048,9 @@ static void binder_stat_br(struct binder_proc *proc,
{
trace_binder_return(cmd);
if (_IOC_NR(cmd) < ARRAY_SIZE(binder_stats.br)) {
- atomic_inc(&binder_stats.br[_IOC_NR(cmd)]);
- atomic_inc(&proc->stats.br[_IOC_NR(cmd)]);
- atomic_inc(&thread->stats.br[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&binder_stats.br[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&proc->stats.br[_IOC_NR(cmd)]);
+ counter_atomic32_inc(&thread->stats.br[_IOC_NR(cmd)]);
}
}
@@ -5841,7 +5842,7 @@ static void print_binder_stats(struct seq_file *m, const char *prefix,
BUILD_BUG_ON(ARRAY_SIZE(stats->bc) !=
ARRAY_SIZE(binder_command_strings));
for (i = 0; i < ARRAY_SIZE(stats->bc); i++) {
- int temp = atomic_read(&stats->bc[i]);
+ int temp = counter_atomic32_read(&stats->bc[i]);
if (temp)
seq_printf(m, "%s%s: %d\n", prefix,
@@ -5851,7 +5852,7 @@ static void print_binder_stats(struct seq_file *m, const char *prefix,
BUILD_BUG_ON(ARRAY_SIZE(stats->br) !=
ARRAY_SIZE(binder_return_strings));
for (i = 0; i < ARRAY_SIZE(stats->br); i++) {
- int temp = atomic_read(&stats->br[i]);
+ int temp = counter_atomic32_read(&stats->br[i]);
if (temp)
seq_printf(m, "%s%s: %d\n", prefix,
@@ -5863,8 +5864,8 @@ static void print_binder_stats(struct seq_file *m, const char *prefix,
BUILD_BUG_ON(ARRAY_SIZE(stats->obj_created) !=
ARRAY_SIZE(stats->obj_deleted));
for (i = 0; i < ARRAY_SIZE(stats->obj_created); i++) {
- int created = atomic_read(&stats->obj_created[i]);
- int deleted = atomic_read(&stats->obj_deleted[i]);
+ int created = counter_atomic32_read(&stats->obj_created[i]);
+ int deleted = counter_atomic32_read(&stats->obj_deleted[i]);
if (created || deleted)
seq_printf(m, "%s%s: active %d total %d\n",
@@ -6054,7 +6055,7 @@ static void print_binder_transaction_log_entry(struct seq_file *m,
int binder_transaction_log_show(struct seq_file *m, void *unused)
{
struct binder_transaction_log *log = m->private;
- unsigned int log_cur = atomic_read(&log->cur);
+ unsigned int log_cur = counter_atomic32_read(&log->cur);
unsigned int count;
unsigned int cur;
int i;
@@ -6124,8 +6125,8 @@ static int __init binder_init(void)
if (ret)
return ret;
- atomic_set(&binder_transaction_log.cur, ~0U);
- atomic_set(&binder_transaction_log_failed.cur, ~0U);
+ counter_atomic32_set(&binder_transaction_log.cur, ~0U);
+ counter_atomic32_set(&binder_transaction_log_failed.cur, ~0U);
binder_debugfs_dir_entry_root = debugfs_create_dir("binder", NULL);
if (binder_debugfs_dir_entry_root)
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 283d3cb9c16e..c77960c01430 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -12,6 +12,7 @@
#include <linux/stddef.h>
#include <linux/types.h>
#include <linux/uidgid.h>
+#include <linux/counters.h>
struct binder_context {
struct binder_node *binder_context_mgr_node;
@@ -136,7 +137,7 @@ struct binder_transaction_log_entry {
};
struct binder_transaction_log {
- atomic_t cur;
+ struct counter_atomic32 cur;
bool full;
struct binder_transaction_log_entry entry[32];
};
--
2.25.1
Powered by blists - more mailing lists