Message-ID: <cfe11fadc3d3e61ce6c7d6f00e1e427edea8a4e3.camel@linux.ibm.com>
Subject: Re: [PATCH RESEND] scsi: libfc: Use refcount_* APIs for reference
 count management
From:   James Bottomley <jejb@linux.ibm.com>
Reply-To: jejb@linux.ibm.com
To:     Deepak R Varma <drv@mailo.com>, Hannes Reinecke <hare@suse.de>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Saurabh Singh Sengar <ssengar@microsoft.com>,
        Praveen Kumar <kumarpraveen@linux.microsoft.com>
Date:   Wed, 01 Mar 2023 14:28:49 -0500
In-Reply-To: <Y/+hVSSFgeV+yPhY@ubun2204.myguest.virtualbox.org>
References: <Y/+hVSSFgeV+yPhY@ubun2204.myguest.virtualbox.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Precedence: bulk

On Thu, 2023-03-02 at 00:32 +0530, Deepak R Varma wrote:
> The atomic_t API based object reference counter management is prone
> to counter value overflows, object use-after-free issues and to
> return puzzling values. The improved refcount_t APIs are designed to
> address these known issues with atomic_t reference counter
> management. This white paper [1] has detailed reasons for moving from
> atomic_t to refcount_t APIs. Hence replace the atomic_* based
> implementation by its refcount_* based equivalent.
> The issue is identified using atomic_as_refcounter.cocci Coccinelle
> semantic patch script.
> 
>         [1] https://arxiv.org/pdf/1710.06175.pdf

Citing long whitepapers in support of a patch isn't helpful to time
pressed reviewers, particularly when it's evident you didn't understand
the paper you cite. The argument in the paper for replacing atomics
with refcounts can be summarized as: if a user can cause a counter
overflow in an atomic_t simply by performing some action from userspace
then that represents a source of potential overflow attacks on the
kernel which should be mitigated by replacing the atomic_t in question
with a refcount_t which is overflow resistant.

What's missing from the quoted changelog is a justification of how a
user could cause an overflow in the ex_refcnt atomic_t.

James