On 02/03/2017 05:19 PM, Christoph Hellwig wrote:
On Fri, Feb 03, 2017 at 02:38:35PM +0100, Hannes Reinecke wrote:
On 02/03/2017 02:31 PM, Christoph Hellwig wrote:
-               if (sg_res_in_use(sfp)) {
+               mutex_lock(&sfp->f_mutex);
+               if (sfp->res_in_use) {
+                       mutex_unlock(&sfp->f_mutex);
                        sg_remove_request(sfp, srp);
                        return -EBUSY;  /* reserve buffer already being used */
                }
+               mutex_unlock(&sfp->f_mutex);
Holding a mutex over a the check of a single scalar doesn't make sense.

It's adds a synchronisation point, doesn't it?
It does, but it doesn't actually protect anything..

But all the other mutex_{un,}locks() do (for instance guarding sg_build_indirect()) and this one provides a synchronization point.

Sorry but I really don't get your point here.

The sole purpose is to guard the reserved list from being altered while blk_rq_map_* or similar functions are in progess (that's what the syzcaller reproducer was doing).

Byte,
    Johannes

Reply via email to