Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 26 Aug 2022 14:08:08 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc:     Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>,
        "J. Bruce Fields" <bfields@redhat.com>,
        Scott Mayhew <smayhew@redhat.com>,
        linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
        linux-nfs@vger.kernel.org
Subject: Re: [PATCH] nfsd: Fix a memory leak in an error handling path
Message-ID: <20220826110808.GE2071@kadam>
References: <122a5729fdcd76e23641c7d1853de2a632f6a742.1661509473.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <122a5729fdcd76e23641c7d1853de2a632f6a742.1661509473.git.christophe.jaillet@wanadoo.fr>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?bpNdnp5iOlq/HZDdhZYY+A1v8wD7zJBbDFi9hALD9nFRJUERSa7VuVyOh1oQ?=
 =?us-ascii?Q?kNIi6/EHeLHAvLmh9PS4Mpps4XXgSj/5eKaqbo9N3yEks0l4LDEr7StnJlDi?=
 =?us-ascii?Q?PK77cmQglu44qJ7xhXgDY2R2alLMDJWzu4mUpbhdVUrW2VS6Zlel/zjDm3Im?=
 =?us-ascii?Q?CuoUTfySyLlOcDGi8cMNbmimyDBQslRxkSvZ876Ee4+a03qo2IV08OqRjtTy?=
 =?us-ascii?Q?qMFyK7dfdVQrI5a3YibEI5Ll8kkfV8Xy5qeKoFktnHh9c52dnvp6StWicj4L?=
 =?us-ascii?Q?sP9ovSWMAwSjq+F3oHWVlwC6yktn90XqljmzxusttYbkJryBxLNqu5L11e+4?=
 =?us-ascii?Q?wIPFORvT9eyhbwQc8MIzwpJWxf7v98dDRJ8iH6v7Xe+IBnUC+cXSWqfFS0o4?=
 =?us-ascii?Q?p0EvXcOgg0MQwl+hfmt1w6jozOS5e6K9nDBlBZ66j+YQlOJXVrnAMM440wuw?=
 =?us-ascii?Q?amxJguTBQ+rFLER7shC+78XxVv+HRU6vmW3+yoYCZwAi0SxFe9KhKrr5twX2?=
 =?us-ascii?Q?PZPDh0dA+xGYJUzG5OHdLXD5peVKymzokQ+IG41995SGFvRwDtlztiad5RXu?=
 =?us-ascii?Q?1ctIl7VUZXzuAywniF68TNFouwjF0P7IP3Ri9iPpjBpq2STSOijZBzw9jflI?=
 =?us-ascii?Q?6Qg6WakTWth0qxZR295cLM5vA6xeWJW5571a70D59KLNFy0Z6c/4zMQ62y2l?=
 =?us-ascii?Q?ssIA1sBe7rnibSHXIgNFZia7h3q0+xECvRAHV6BieT/GBrxW/xde6b9HFZll?=
 =?us-ascii?Q?RdvMDKgPi2LsrELzoGcUvG/h2zJHV1S9UZYzPtGuRb3nENfftIjgssIQ0Eef?=
 =?us-ascii?Q?R6qc2OtOgNwCP8FkEgLyKXv3YiuafA6Mha6lq9egWxyjB5sxjmu2cTqTsWy4?=
 =?us-ascii?Q?bV1fk2JFrSSwAhQy9pC1qAslXzjzRUqh1JLlpnigfdVtm0o2zjZ01LnclJQF?=
 =?us-ascii?Q?Oyk7aXVEJwwCnNWlGBwEgCGd2RFs8LBHQ+4zYFSKNpTVGMMRmTtSOFPOiBUG?=
 =?us-ascii?Q?wBS9bBy+Ak8OdX3PzTD9ZZnI2CBijHyQI8OKutVn8mx7Nc2Zurt0xsRWaPLY?=
 =?us-ascii?Q?baL9c0ps/A1noquhUBB5mraxtWNYza1QIJTK6iRJ0emHCkSbkS5yl3xcbvfQ?=
 =?us-ascii?Q?Eupdis/jMKBkuXJ8BpXxgtjkrp0gPm6I1yG4qXHrbkLEM/ftBrDiuiUYJja2?=
 =?us-ascii?Q?ybtliFPXgktO/xXA8h4Ll8DxscVUnDz83YBJkj6IYZnp9NbGCG2QWK3Hd/6y?=
 =?us-ascii?Q?fBjIBVL5yRJ4xgHTYSHkX8awYmhK6jgsSftYESOcRjHHMdulyS68O3EqSH2K?=
 =?us-ascii?Q?tHkVGP0erzrfXagQ6l4HJjydPySMkBtQAPZ3wFMLEgmrtatyM/4hFm6LfvA7?=
 =?us-ascii?Q?VWNy3+8Ygt7q66YlNkuXLhYRGl55YMN2c8TJNarBvHR436uk/O+NLzh8Tqxo?=
 =?us-ascii?Q?vgeD3ZF3MbYS9rR1saCyuNJCcilTB20dv75AJ8BgStRHtHGxduGPcYII+X6j?=
 =?us-ascii?Q?35Es8dI8+HuvgrwtfKb+LpHuTAW2UXTB27KxxLvO4ntF1cnTh6EN2p3Q+Yyj?=
 =?us-ascii?Q?QwRznVa3EBj2r9kf9t2IBArmXCRCXi3R3v0mau9nLHozlAH9nio4B2ZMDzrB?=
 =?us-ascii?Q?9g=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 675b082e-a31c-4c35-4b9e-08da87534e19
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Aug 2022 11:08:29.1474
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: /RLaYWc7jI2qCDyd7uK7Z/dedQTKIYHf5YhsmhxQp0G0GHryx6flhiuPSTPZUP1ZfWjv6pkfzn2IqdQ0DqWJg2lgeOGga+MP+Mixmq5S4kg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR10MB5764
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-26_04,2022-08-25_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 phishscore=0 spamscore=0
 bulkscore=0 adultscore=0 mlxscore=0 malwarescore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2207270000
 definitions=main-2208260046
X-Proofpoint-ORIG-GUID: P2gbWQPwDn_TBaOFyQvwlGWYnRUsMH48
X-Proofpoint-GUID: P2gbWQPwDn_TBaOFyQvwlGWYnRUsMH48
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

On Fri, Aug 26, 2022 at 12:24:54PM +0200, Christophe JAILLET wrote:
> If this memdup_user() call fails, the memory allocated in a previous call
> a few lines above should be freed. Otherwise it leaks.
> 
> Fixes: 6ee95d1c8991 ("nfsd: add support for upcall version 2")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Speculative, untested.
> ---
>  fs/nfsd/nfs4recover.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index b29d27eaa8a6..248ff9f4141c 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -815,8 +815,10 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
>  				princhash.data = memdup_user(
>  						&ci->cc_princhash.cp_data,
>  						princhashlen);
> -				if (IS_ERR_OR_NULL(princhash.data))
> +				if (IS_ERR_OR_NULL(princhash.data)) {
> +					kfree(name.data);
>  					return -EFAULT;

This comment is not directed at you and is not related to your patch.
But memdup_user() never returns NULL, only error pointers.  I wrote a
fifteen page blog entry about NULL vs error pointers the other week.
https://staticthinking.wordpress.com/2022/08/01/mixing-error-pointers-and-null/
This should propagate the error code from memdup_user() instead of
-EFAULT.

regards,
dan carpenter