Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit a9caf93

Browse files
authored
Merge pull request #328 from sysprog21/drop-this-module
Remove unsafe try_module_get(THIS_MODULE)
2 parents 78a7265 + b859b8b commit a9caf93

File tree

7 files changed

+9
-46
lines changed

7 files changed

+9
-46
lines changed

‎examples/chardev.c‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ static int device_open(struct inode *inode, struct file *file)
9696
return -EBUSY;
9797

9898
sprintf(msg, "I already told you %d times Hello world!\n", counter++);
99-
try_module_get(THIS_MODULE);
10099

101100
return 0;
102101
}
@@ -107,11 +106,6 @@ static int device_release(struct inode *inode, struct file *file)
107106
/* We're now ready for our next caller */
108107
atomic_set(&already_open, CDEV_NOT_USED);
109108

110-
/* Decrement the usage count, or else once you opened the file, you will
111-
* never get rid of the module.
112-
*/
113-
module_put(THIS_MODULE);
114-
115109
return 0;
116110
}
117111

‎examples/chardev2.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,13 @@ static int device_open(struct inode *inode, struct file *file)
4040
{
4141
pr_info("device_open(%p)\n", file);
4242

43-
try_module_get(THIS_MODULE);
4443
return 0;
4544
}
4645

4746
static int device_release(struct inode *inode, struct file *file)
4847
{
4948
pr_info("device_release(%p,%p)\n", inode, file);
5049

51-
module_put(THIS_MODULE);
5250
return 0;
5351
}
5452

‎examples/procfs3.c‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,10 @@ static ssize_t procfs_write(struct file *file, const char __user *buffer,
5252
}
5353
static int procfs_open(struct inode *inode, struct file *file)
5454
{
55-
try_module_get(THIS_MODULE);
5655
return 0;
5756
}
5857
static int procfs_close(struct inode *inode, struct file *file)
5958
{
60-
module_put(THIS_MODULE);
6159
return 0;
6260
}
6361

‎examples/sleep.c‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ static int module_open(struct inode *inode, struct file *file)
9595
/* Try to get without blocking */
9696
if (!atomic_cmpxchg(&already_open, 0, 1)) {
9797
/* Success without blocking, allow the access */
98-
try_module_get(THIS_MODULE);
9998
return 0;
10099
}
101100
/* If the file's flags include O_NONBLOCK, it means the process does not
@@ -106,12 +105,6 @@ static int module_open(struct inode *inode, struct file *file)
106105
if (file->f_flags & O_NONBLOCK)
107106
return -EAGAIN;
108107

109-
/* This is the correct place for try_module_get(THIS_MODULE) because if
110-
* a process is in the loop, which is within the kernel module,
111-
* the kernel module must not be removed.
112-
*/
113-
try_module_get(THIS_MODULE);
114-
115108
while (atomic_cmpxchg(&already_open, 0, 1)) {
116109
int i, is_sig = 0;
117110

@@ -132,15 +125,7 @@ static int module_open(struct inode *inode, struct file *file)
132125
is_sig = current->pending.signal.sig[i] & ~current->blocked.sig[i];
133126

134127
if (is_sig) {
135-
/* It is important to put module_put(THIS_MODULE) here, because
136-
* for processes where the open is interrupted there will never
137-
* be a corresponding close. If we do not decrement the usage
138-
* count here, we will be left with a positive usage count
139-
* which we will have no way to bring down to zero, giving us
140-
* an immortal module, which can only be killed by rebooting
141-
* the machine.
142-
*/
143-
module_put(THIS_MODULE);
128+
/* Return -EINTR if we got a signal */
144129
return -EINTR;
145130
}
146131
}
@@ -163,8 +148,6 @@ static int module_close(struct inode *inode, struct file *file)
163148
*/
164149
wake_up(&waitq);
165150

166-
module_put(THIS_MODULE);
167-
168151
return 0; /* success */
169152
}
170153

‎examples/static_key.c‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ static int device_open(struct inode *inode, struct file *file)
100100
pr_alert("do unlikely thing\n");
101101
pr_info("fastpath 2\n");
102102

103-
try_module_get(THIS_MODULE);
104-
105103
return 0;
106104
}
107105

@@ -113,12 +111,6 @@ static int device_release(struct inode *inode, struct file *file)
113111
/* We are now ready for our next caller. */
114112
atomic_set(&already_open, CDEV_NOT_USED);
115113

116-
/**
117-
* Decrement the usage count, or else once you opened the file, you will
118-
* never get rid of the module.
119-
*/
120-
module_put(THIS_MODULE);
121-
122114
return 0;
123115
}
124116

‎examples/vinput.c‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,6 @@ static void vinput_destroy_vdevice(struct vinput *vinput)
157157
clear_bit(vinput->id, vinput_ids);
158158
spin_unlock(&vinput_lock);
159159

160-
module_put(THIS_MODULE);
161-
162160
kfree(vinput);
163161
}
164162

@@ -182,8 +180,6 @@ static struct vinput *vinput_alloc_vdevice(void)
182180
return ERR_PTR(-ENOMEM);
183181
}
184182

185-
try_module_get(THIS_MODULE);
186-
187183
spin_lock_init(&vinput->lock);
188184

189185
spin_lock(&vinput_lock);
@@ -217,7 +213,6 @@ static struct vinput *vinput_alloc_vdevice(void)
217213
list_del(&vinput->list);
218214
fail_id:
219215
spin_unlock(&vinput_lock);
220-
module_put(THIS_MODULE);
221216
kfree(vinput);
222217

223218
return ERR_PTR(err);

‎lkmpg.tex‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,16 +1095,17 @@ \subsection{Unregistering A Device}
10951095
You can see what its value is by looking at the 3rd field with the command \sh|cat /proc/modules| or \sh|lsmod|.
10961096
If this number isn't zero, \sh|rmmod| will fail.
10971097
Note that you do not have to check the counter within \cpp|cleanup_module| because the check will be performed for you by the system call \cpp|sys_delete_module|, defined in \src{include/linux/syscalls.h}.
1098-
You should not use this counter directly, but there are functions defined in \src{include/linux/module.h} which let you increase, decrease and display this counter:
1098+
You should not use this counter directly, but there are functions defined in \src{include/linux/module.h} which let you display this counter:
10991099

11001100
\begin{itemize}
1101-
\item \cpp|try_module_get(THIS_MODULE)|: Increment the reference count of current module.
1102-
\item \cpp|module_put(THIS_MODULE)|: Decrement the reference count of current module.
11031101
\item \cpp|module_refcount(THIS_MODULE)|: Return the value of reference count of current module.
11041102
\end{itemize}
11051103

1106-
It is important to keep the counter accurate; if you ever lose track of the correct usage count, you will never be able to unload the module; it is now reboot time.
1107-
This is bound to happen to you sooner or later during a module's development.
1104+
Note: The use of \cpp|try_module_get(THIS_MODULE)| and \cpp|module_put(THIS_MODULE)| within a module's own code is considered unsafe and should be avoided.
1105+
The kernel automatically manages the reference count when file operations are in progress,
1106+
so manual reference counting is unnecessary and can lead to race conditions.
1107+
For a deeper understanding of when and how to properly use module reference counting,
1108+
see \url{https://stackoverflow.com/questions/1741415/linux-kernel-modules-when-to-use-try-module-get-module-put}.
11081109

11091110
\subsection{chardev.c}
11101111
\label{sec:chardev_c}
@@ -1152,6 +1153,8 @@ \section{The /proc Filesystem}
11521153
The inode contains information about the file, for example the file's permissions, together with a pointer to the disk location or locations where the file's data can be found.
11531154

11541155
Because we do not get called when the file is opened or closed, there is nowhere for us to put \cpp|try_module_get| and \cpp|module_put| in this module, and if the file is opened and then the module is removed, there is no way to avoid the consequences.
1156+
The kernel's automatic reference counting for file operations helps prevent module removal while files are in use,
1157+
but \verb|/proc| files require careful handling due to their different lifecycle.
11551158

11561159
Here is a simple example showing how to use a \verb|/proc| file.
11571160
This is the HelloWorld for the \verb|/proc| filesystem.

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /