On Sun, Aug 23, 2020 at 8:28 AM Madhuparna Bhowmik <[email protected]> wrote: > > sl->xhead is modified in both x25_asy_change_mtu() and > x25_asy_write_wakeup(). However, sl->lock is not held in > x25_asy_write_wakeup(). So, I am not sure if it is indeed possible to > have a race between these two functions. If it is possible that these > two functions can execute in parallel then the lock should be held in > x25_asy_write_wakeup() as well. Please let me know if this race is > possible.
I think you are right. These two functions do race with each other. There seems to be nothing preventing them from racing. We need to hold the lock in x25_asy_write_wakeup to prevent it from racing with x25_asy_change_mtu. By the way, I think this driver has bigger problems. We can see that these function pairs are not symmetric with one another in what they do: "x25_asy_alloc" and "x25_asy_free"; "x25_asy_open" and "x25_asy_close"; "x25_asy_open_tty" and "x25_asy_close_tty"; "x25_asy_netdev_ops.ndo_open" and "x25_asy_netdev_ops.ndo_stop". This not only makes the code messy, but also makes the actual runtime behavior buggy. I'm planning to fix this with this change: https://github.com/hyanggi/linux/commit/66387f229168014024117d50ade01092e3c9932c Please take a look if you are interested. Thanks! Thanks, Xie He
