misc: uidstat: avoid create_stat() race and blockage.
authorJP Abgrall <jpa@google.com>
Thu, 30 May 2013 22:31:17 +0000 (15:31 -0700)
committerArve Hjønnevåg <arve@android.com>
Mon, 1 Jul 2013 21:16:26 +0000 (14:16 -0700)
* create_stat() race would lead to:
  [   58.132324] proc_dir_entry 'uid_stat/10061' already registered

* blocking kmalloc reported by sbranden
 tcp_read_sock()
  uid_stat_tcp_rcv()
    create_stat()
      kmalloc(GFP_KERNEL)

Signed-off-by: JP Abgrall <jpa@google.com>
drivers/misc/uid_stat.c

index 2141124a6c1291e2ace75a465c5c3b803364531c..509822c81e97b74863db6d5247f659cf06c3eb41 100644 (file)
@@ -38,17 +38,13 @@ struct uid_stat {
 };
 
 static struct uid_stat *find_uid_stat(uid_t uid) {
-       unsigned long flags;
        struct uid_stat *entry;
 
-       spin_lock_irqsave(&uid_lock, flags);
        list_for_each_entry(entry, &uid_list, link) {
                if (entry->uid == uid) {
-                       spin_unlock_irqrestore(&uid_lock, flags);
                        return entry;
                }
        }
-       spin_unlock_irqrestore(&uid_lock, flags);
        return NULL;
 }
 
@@ -90,13 +86,10 @@ static int tcp_rcv_read_proc(char *page, char **start, off_t off,
 
 /* Create a new entry for tracking the specified uid. */
 static struct uid_stat *create_stat(uid_t uid) {
-       unsigned long flags;
-       char uid_s[32];
        struct uid_stat *new_uid;
-       struct proc_dir_entry *entry;
-
        /* Create the uid stat struct and append it to the list. */
-       if ((new_uid = kmalloc(sizeof(struct uid_stat), GFP_KERNEL)) == NULL)
+       new_uid = kmalloc(sizeof(struct uid_stat), GFP_ATOMIC);
+       if (!new_uid)
                return NULL;
 
        new_uid->uid = uid;
@@ -104,11 +97,15 @@ static struct uid_stat *create_stat(uid_t uid) {
        atomic_set(&new_uid->tcp_rcv, INT_MIN);
        atomic_set(&new_uid->tcp_snd, INT_MIN);
 
-       spin_lock_irqsave(&uid_lock, flags);
        list_add_tail(&new_uid->link, &uid_list);
-       spin_unlock_irqrestore(&uid_lock, flags);
+       return new_uid;
+}
 
-       sprintf(uid_s, "%d", uid);
+static void create_stat_proc(struct uid_stat *new_uid)
+{
+       char uid_s[32];
+       struct proc_dir_entry *entry;
+       sprintf(uid_s, "%d", new_uid->uid);
        entry = proc_mkdir(uid_s, parent);
 
        /* Keep reference to uid_stat so we know what uid to read stats from. */
@@ -117,17 +114,31 @@ static struct uid_stat *create_stat(uid_t uid) {
 
        create_proc_read_entry("tcp_rcv", S_IRUGO, entry, tcp_rcv_read_proc,
                (void *) new_uid);
+}
 
-       return new_uid;
+static struct uid_stat *find_or_create_uid_stat(uid_t uid)
+{
+       struct uid_stat *entry;
+       unsigned long flags;
+       spin_lock_irqsave(&uid_lock, flags);
+       entry = find_uid_stat(uid);
+       if (entry) {
+               spin_unlock_irqrestore(&uid_lock, flags);
+               return entry;
+       }
+       entry = create_stat(uid);
+       spin_unlock_irqrestore(&uid_lock, flags);
+       if (entry)
+               create_stat_proc(entry);
+       return entry;
 }
 
 int uid_stat_tcp_snd(uid_t uid, int size) {
        struct uid_stat *entry;
        activity_stats_update();
-       if ((entry = find_uid_stat(uid)) == NULL &&
-               ((entry = create_stat(uid)) == NULL)) {
-                       return -1;
-       }
+       entry = find_or_create_uid_stat(uid);
+       if (!entry)
+               return -1;
        atomic_add(size, &entry->tcp_snd);
        return 0;
 }
@@ -135,10 +146,9 @@ int uid_stat_tcp_snd(uid_t uid, int size) {
 int uid_stat_tcp_rcv(uid_t uid, int size) {
        struct uid_stat *entry;
        activity_stats_update();
-       if ((entry = find_uid_stat(uid)) == NULL &&
-               ((entry = create_stat(uid)) == NULL)) {
-                       return -1;
-       }
+       entry = find_or_create_uid_stat(uid);
+       if (!entry)
+               return -1;
        atomic_add(size, &entry->tcp_rcv);
        return 0;
 }