inotify: fix double free/corruption of stuct user

On an error path in inotify_init1 a normal user can trigger a double
free of struct user. This is a regression introduced by a2ae4cc9a16e
("inotify: stop kernel memory leak on file creation failure").

We fix this by making sure that if a group exists the user reference is
dropped when the group is cleaned up. We should not explictly drop the
reference on error and also drop the reference when the group is cleaned
up.

The new lifetime rules are that an inotify group lives from
inotify_new_group to the last fsnotify_put_group. Since the struct user
and inotify_devs are directly tied to this lifetime they are only
changed/updated in those two locations. We get rid of all special
casing of struct user or user->inotify_devs.

Signed-off-by: Eric Paris <eparis@redhat.com>
Cc: stable@kernel.org (2.6.37 and up)
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Eric Paris and committed by

Linus Torvalds 15 years ago d0de4dc5 623dda65

+14 -26

2 changed files

expand all

notify

inotify

inotify_fsnotify.c

inotify_user.c

fs/notify/inotify/inotify_fsnotify.c

··· 198 198 idr_for_each(&group->inotify_data.idr, idr_callback, group); 199 199 idr_remove_all(&group->inotify_data.idr); 200 200 idr_destroy(&group->inotify_data.idr); 201 + atomic_dec(&group->inotify_data.user->inotify_devs); 201 202 free_uid(group->inotify_data.user); 202 203 } 203 204

+13 -26

fs/notify/inotify/inotify_user.c

··· 290 290 static int inotify_release(struct inode *ignored, struct file *file) 291 291 { 292 292 struct fsnotify_group *group = file->private_data; 293 - struct user_struct *user = group->inotify_data.user; 294 293 295 294 pr_debug("%s: group=%p\n", __func__, group); 296 295 ··· 297 298 298 299 /* free this group, matching get was inotify_init->fsnotify_obtain_group */ 299 300 fsnotify_put_group(group); 300 - 301 - atomic_dec(&user->inotify_devs); 302 301 303 302 return 0; 304 303 } ··· 694 697 return ret; 695 698 } 696 699 697 - static struct fsnotify_group *inotify_new_group(struct user_struct *user, unsigned int max_events) 700 + static struct fsnotify_group *inotify_new_group(unsigned int max_events) 698 701 { 699 702 struct fsnotify_group *group; 700 703 ··· 707 710 spin_lock_init(&group->inotify_data.idr_lock); 708 711 idr_init(&group->inotify_data.idr); 709 712 group->inotify_data.last_wd = 0; 710 - group->inotify_data.user = user; 711 713 group->inotify_data.fa = NULL; 714 + group->inotify_data.user = get_current_user(); 715 + 716 + if (atomic_inc_return(&group->inotify_data.user->inotify_devs) > 717 + inotify_max_user_instances) { 718 + fsnotify_put_group(group); 719 + return ERR_PTR(-EMFILE); 720 + } 712 721 713 722 return group; 714 723 } ··· 724 721 SYSCALL_DEFINE1(inotify_init1, int, flags) 725 722 { 726 723 struct fsnotify_group *group; 727 - struct user_struct *user; 728 724 int ret; 729 725 730 726 /* Check the IN_* constants for consistency. */ ··· 733 731 if (flags & ~(IN_CLOEXEC | IN_NONBLOCK)) 734 732 return -EINVAL; 735 733 736 - user = get_current_user(); 737 - if (unlikely(atomic_read(&user->inotify_devs) >= 738 - inotify_max_user_instances)) { 739 - ret = -EMFILE; 740 - goto out_free_uid; 741 - } 742 - 743 734 /* fsnotify_obtain_group took a reference to group, we put this when we kill the file in the end */ 744 - group = inotify_new_group(user, inotify_max_queued_events); 745 - if (IS_ERR(group)) { 746 - ret = PTR_ERR(group); 747 - goto out_free_uid; 748 - } 749 - 750 - atomic_inc(&user->inotify_devs); 735 + group = inotify_new_group(inotify_max_queued_events); 736 + if (IS_ERR(group)) 737 + return PTR_ERR(group); 751 738 752 739 ret = anon_inode_getfd("inotify", &inotify_fops, group, 753 740 O_RDONLY | flags); 754 - if (ret >= 0) 755 - return ret; 741 + if (ret < 0) 742 + fsnotify_put_group(group); 756 743 757 - fsnotify_put_group(group); 758 - atomic_dec(&user->inotify_devs); 759 - out_free_uid: 760 - free_uid(user); 761 744 return ret; 762 745 } 763 746

Configure Feed

Configure Feed