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

On Wed, Mar 01, 2023 at 02:28:49PM -0500, James Bottomley wrote:
> 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.

Thank you very much James for the review comments. I truly appreciate your time
and guidance. I will study your feedback and send in a revision with necessary
update to patch log.

Regards,
./drv

> 
> James
>