The rule of thumb is to include the headers we are the direct user of.
In particular, if we need an atomic API, we include <linux/atomic.h>.
On the other hand we should not use headers for no reason. In particular,
if we are not doing any IRQ job, why is the <linux/irq.h> included?
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
platform_driver_register() macro already does the correct assignment for
the owner of the driver.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Since sys_open is deprecated and some architectures don't support it.
We switch the implementation to sys_openat.
Moreover, in some architectures like x86-64, the prototype of syscall,
for example, openat(), might have been changed to struct pt_regs [1]
but we cannot promise that so support the two types
(sys_openat and pt_regs).
Also, to prevent other untraced tasks print out the information, add
the uid checking in our_sys_openat().
[1] https://lore.kernel.org/lkml/20180405095307.3730-1-linux@dominikbrodowski.net/Close#159
It failed silently when crypto_alloc_tfm() failed, so add an error
message to inform the developer to enable sha256 algorithm support.
Signed-off-by: Yu Chien Peter Lin <peterlin.tw@pm.me>
To make sure the behavior of the read and write operations are correct
with offset, update it each time. Also, since it is using the offset,
modify the part of read for removing unnecessary variable.
Since The length of the message buffer is BUF_LEN. When writing the
BUF_LEN length of the string it will overwrite the last character
(usually it is '\0' from the initialization). And, because the read
operation uses the character in the message buffer ('\0') to stop the
read loop. It will cause the read operation will read out of the
message buffer when the length parameter of read() is not lower than
or equal to BUF_LEN. So add one more byte space to avoid this problem.
The description of the attribute was added in sysfs section and referenced in vinput section.
In vinput section, just described the class_attribute and some macros about sysfs class.
Add file name at vinput-related examples begin.
After forking, Each file descriptor in the child refers to the same
open file description as the parent. So when calling open() before
fork(), the child can access the device file without checking by
exclusive access in device_open(). It may cause race conditions
in device_ioctl().
Because of that, it is unnecessary to check the multiple access
in device_open(). It just needs check in device_ioctl(), since
read(), write(), seek() system call are atomic [1][2].
Related discussion:
- https://github.com/sysprog21/lkmpg/issues/148
[1] https://lore.kernel.org/lkml/53022DB1.4070805@gmail.com/
[2] https://www.kernel.org/doc/html/latest/filesystems/files.htmlClose#148
Presently, the representation of showing which version will use the
method to find out the system call table is vague.
Related discussion:
* sysprog21/lkmpg: https://github.com/sysprog21/lkmpg/pull/142
Change the "alloc_ret" and "cdev_ret" initial values to non-zero.
According to the source code, "alloc_chrdev_region" and "cdev_add"
return zero on success, and negative code on failure.
So, if the "alloc_chrdev_region" failed, the if condition becomes true,
then we will jump to the label "error" by goto, checking each return
value whether is a success state from both functions mentioned above
and dealing with it properly for exiting the process.
However, it checks the success state by comparing the return value
with zero (means success), and we got "cdev_ret == 0" is true from
the initial value zero, while we didn't execute "cdev_add" yet.
Hence, there was a logic error when the initial value is zero.
Co-authored-by: NOVBobLee <defru04002@gamil.com>
Previously ioctl.c is a userspace program for chardev2.c and chardev.h [1].
But now, this file is an independent kernel module, and the original code
disappear.
This patch adds back the original userspace code and renames it to
userspace_ioctl.c.
[1] https://tldp.org/LDP/lkmpg/2.4/html/x856.html
Smatch[1][2] is a pluggable static analysis for C. It may help us find
out the potential problem of the example code.
Doing with smatch, if set the --file-output flag, it will generate the
{}.c.smatch report for each c file. This will make a little bit
complicated to collect all the report messages. So, here we stay at the
default setting, stdout for the smatch messages.
For more information, see:
- https://lwn.net/Articles/696624/
- https://elinux.org/images/d/d3/Bargmann.pdf
Also, fix the warning from Smatch:
Smatch failed: 1 warning(s), 1 error(s)
lkmpg/examples/procfs2.c:57 procfile_write() error: buffer overflow 'procfs_buffer' 1024 <= 1024
lkmpg/examples/kbleds.c:58 kbleds_init() warn: argument 5 to %lx specifier is cast from pointer
Furthermore, the effect of the write operation in procfs2.c is too
implied. So after writing, print the buffer every time.
Close#122
[1] https://github.com/error27/smatch
[2] https://repo.or.cz/w/smatch.git
Since Linux v3.14, the read, write and seek operations of "struct file" are
guaranteed for thread safety [1][2]. This patch added an explanation.
Here are the potential problems:
chardev.c:
- Move the "msg_ptr" pointer into the read function to remove unnecessary usage.
- List the clear states of "already_open" by using mnemonic enumeration.
chardev2.c:
- The "buffer" in the write function is user space data. It cannot use in the
kernel space.
- Reduce the redundant type transformation.
- List the states of "already_open". Same as chardev.c.
[1] https://lore.kernel.org/lkml/20140303210359.26624.qmail@science.horizon.com/T/#u
[2] 9c225f2655
Cppcheck[1] is integrated into CI pipeline for running static analysis.
However, Cppcheck is known to report false-positive, and we have to
suppress some warnings in advance.
[1] https://cppcheck.sourceforge.io/
The "alloc_chrdev_region" function will dynamically choose the
major number and store it at "dev". It is unnecessary to initialize
the "dev" before the "alloc_chrdev_region" function.
In file {chardev,chardev2,sleep}.c, the variable to determine
the exclusive access was of integer type, which led to race
condition.
This patch rewrote the above with atomic CAS respectively
to eliminate the race.
Close#93
chardev2.c demonstrates the ioctl operation with static major
number MAJOR_NUM, but there also exists "Major," the dynamic
one, which results in registration and deregistration on different
device. Once the module remove, it cannot insert again:
$ sudo insmod chardev2.ko
$ sudo rmmod chardev2
$ cat /proc/devices
Character devices:
...
100 char_dev
$ sudo insmod chardev2.ko
insmod: ERROR: could not insert module chardev2.ko: Device or resource busy
This patch removed the use of dynamic major number.
The latter might have been used along with printk() in an earlier
version, but pr_info() is getting used consistently so adjust the
#include comments accordingly.
For the avoidance of doubt, pr_info() actually comes from printk.h,
which gets #include'd by kernel.h.
MODULE_SUPPORTED_DEVICE is remove from upstream since March 17, 2021.
See linux.git commit 6417f03
Reported by Niklas Lantau <niklaslantau@gmail.com>
Close#61
In rendered HTML, the line number should be right aligned, and
code should be left aligned accordingly.
In addition, this patch added the basic build instructions, so that
someone can generated the PDF and HTML files.
Close#44
