Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210120102837.23663-1-xie.he.0141@gmail.com>
In-Reply-To: <20210120102837.23663-1-xie.he.0141@gmail.com>
From:   Xie He <xie.he.0141@gmail.com>
Date:   Wed, 20 Jan 2021 12:42:17 -0800
Message-ID: <CAJht_EOdpq5wQfcFROpx587kCZ9dGRz6kesyPVhgZyKdoqS8Cg@mail.gmail.com>
Subject: Re: [PATCH net v4] net: lapb: Add locking to the lapb module
To:     "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Linux X25 <linux-x25@vger.kernel.org>,
        Linux Kernel Network Developers <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Martin Schiller <ms@dev.tdt.de>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

With this patch, there is still a problem that lapb_unregister may run
concurrently with other LAPB API functions (such as
lapb_data_received). This other LAPB API function can get the
lapb->lock after lapb->lock is released by lapb_unregister, and
continue to do its work. This is not correct.

We can fix this problem by adding a new field "bool stop" to "struct
lapb_cb" (just like "bool t1timer_stop, t2timer_stop"), and make every
API function abort whenever it sees lapb->stop == true after getting
the lock.

Alternatively we can also require the callers (the LAPB drivers) to
never call lapb_unregister concurrently with other LAPB APIs. They
should make sure all LAPB API functions are only called after
lapb_register ends and before lapb_unregister starts. This is a
reasonable requirement, because if they don't follow this requirement,
even if we do the fix in the LAPB module (as said above), the LAPB
driver will still get the "LAPB_BADTOKEN" error from the LAPB module.
This is not desirable and I think LAPB drivers should avoid this from
happening.

So I think this problem may not need to be fixed here in the LAPB
module because the LAPB drivers should deal with this problem anyway.

Please feel free to share your comment. Thanks!