| Message ID | 1509087616-28222-1-git-send-email-girish.moodalbail@oracle.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | tap: reference to KVA of an unloaded module causes kernel panic | expand |
From: Girish Moodalbail <girish.moodalbail@oracle.com> Date: Fri, 27 Oct 2017 00:00:16 -0700 > The commit 9a393b5d5988 ("tap: tap as an independent module") created a > separate tap module that implements tap functionality and exports > interfaces that will be used by macvtap and ipvtap modules to create > create respective tap devices. > > However, that patch introduced a regression wherein the modules macvtap > and ipvtap can be removed (through modprobe -r) while there are > applications using the respective /dev/tapX devices. These applications > cause kernel to hold reference to /dev/tapX through 'struct cdev > macvtap_cdev' and 'struct cdev ipvtap_dev' defined in macvtap and ipvtap > modules respectively. So, when the application is later closed the > kernel panics because we are referencing KVA that is present in the > unloaded modules. > > ----------8<------- Example ----------8<---------- > $ sudo ip li add name mv0 link enp7s0 type macvtap > $ sudo ip li show mv0 |grep mv0| awk -e '{print $1 $2}' > 14:mv0@enp7s0: > $ cat /dev/tap14 & > $ lsmod |egrep -i 'tap|vlan' > macvtap 16384 0 > macvlan 24576 1 macvtap > tap 24576 3 macvtap > $ sudo modprobe -r macvtap > $ fg > cat /dev/tap14 > ^C > > <...system panics...> > BUG: unable to handle kernel paging request at ffffffffa038c500 > IP: cdev_put+0xf/0x30 > ----------8<-----------------8<---------- > > The fix is to set cdev.owner to the module that creates the tap device > (either macvtap or ipvtap). With this set, the operations (in > fs/char_dev.c) on char device holds and releases the module through > cdev_get() and cdev_put() and will not allow the module to unload > prematurely. > > Fixes: 9a393b5d5988ea4e (tap: tap as an independent module) > Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com> Applied and queued up for -stable.
diff --git a/drivers/net/ipvlan/ipvtap.c b/drivers/net/ipvlan/ipvtap.c index 5dea206..0bcc07f 100644 --- a/drivers/net/ipvlan/ipvtap.c +++ b/drivers/net/ipvlan/ipvtap.c @@ -197,8 +197,8 @@ static int ipvtap_init(void) { int err; - err = tap_create_cdev(&ipvtap_cdev, &ipvtap_major, "ipvtap"); - + err = tap_create_cdev(&ipvtap_cdev, &ipvtap_major, "ipvtap", + THIS_MODULE); if (err) goto out1; diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index c2d0ea2..cba5cb3 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -204,8 +204,8 @@ static int macvtap_init(void) { int err; - err = tap_create_cdev(&macvtap_cdev, &macvtap_major, "macvtap"); - + err = tap_create_cdev(&macvtap_cdev, &macvtap_major, "macvtap", + THIS_MODULE); if (err) goto out1; diff --git a/drivers/net/tap.c b/drivers/net/tap.c index 98ee6cc..1b10fcc 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -1249,8 +1249,8 @@ static int tap_list_add(dev_t major, const char *device_name) return 0; } -int tap_create_cdev(struct cdev *tap_cdev, - dev_t *tap_major, const char *device_name) +int tap_create_cdev(struct cdev *tap_cdev, dev_t *tap_major, + const char *device_name, struct module *module) { int err; @@ -1259,6 +1259,7 @@ int tap_create_cdev(struct cdev *tap_cdev, goto out1; cdev_init(tap_cdev, &tap_fops); + tap_cdev->owner = module; err = cdev_add(tap_cdev, *tap_major, TAP_NUM_DEVS); if (err) goto out2; diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h index 4837157..9ae41cd 100644 --- a/include/linux/if_tap.h +++ b/include/linux/if_tap.h @@ -73,8 +73,8 @@ struct tap_queue { int tap_get_minor(dev_t major, struct tap_dev *tap); void tap_free_minor(dev_t major, struct tap_dev *tap); int tap_queue_resize(struct tap_dev *tap); -int tap_create_cdev(struct cdev *tap_cdev, - dev_t *tap_major, const char *device_name); +int tap_create_cdev(struct cdev *tap_cdev, dev_t *tap_major, + const char *device_name, struct module *module); void tap_destroy_cdev(dev_t major, struct cdev *tap_cdev); #endif /*_LINUX_IF_TAP_H_*/
The commit 9a393b5d5988 ("tap: tap as an independent module") created a separate tap module that implements tap functionality and exports interfaces that will be used by macvtap and ipvtap modules to create create respective tap devices. However, that patch introduced a regression wherein the modules macvtap and ipvtap can be removed (through modprobe -r) while there are applications using the respective /dev/tapX devices. These applications cause kernel to hold reference to /dev/tapX through 'struct cdev macvtap_cdev' and 'struct cdev ipvtap_dev' defined in macvtap and ipvtap modules respectively. So, when the application is later closed the kernel panics because we are referencing KVA that is present in the unloaded modules. ----------8<------- Example ----------8<---------- $ sudo ip li add name mv0 link enp7s0 type macvtap $ sudo ip li show mv0 |grep mv0| awk -e '{print $1 $2}' 14:mv0@enp7s0: $ cat /dev/tap14 & $ lsmod |egrep -i 'tap|vlan' macvtap 16384 0 macvlan 24576 1 macvtap tap 24576 3 macvtap $ sudo modprobe -r macvtap $ fg cat /dev/tap14 ^C <...system panics...> BUG: unable to handle kernel paging request at ffffffffa038c500 IP: cdev_put+0xf/0x30 ----------8<-----------------8<---------- The fix is to set cdev.owner to the module that creates the tap device (either macvtap or ipvtap). With this set, the operations (in fs/char_dev.c) on char device holds and releases the module through cdev_get() and cdev_put() and will not allow the module to unload prematurely. Fixes: 9a393b5d5988ea4e (tap: tap as an independent module) Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com> --- drivers/net/ipvlan/ipvtap.c | 4 ++-- drivers/net/macvtap.c | 4 ++-- drivers/net/tap.c | 5 +++-- include/linux/if_tap.h | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-)