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;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Dan Carpenter <dan.carpenter@linaro.org>
CC:     Thadeu Lima de Souza Cascardo <cascardo@canonical.com>,
        Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>,
        Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna@kernel.org>,
        Jeff Layton <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] net: avoid double iput when sock_alloc_file fails
Thread-Topic: [PATCH] net: avoid double iput when sock_alloc_file fails
Thread-Index: AQHZfbLSlzKle740mUCBeF98PrPl7q9KQf+A
Date:   Thu, 4 May 2023 15:40:15 +0000
Message-ID: <53A81159-DD6C-4A5E-994F-49A536F7BD31@oracle.com>
References: <20230307173707.468744-1-cascardo@canonical.com>
 <95e36d70-7841-4e7d-960a-116b8595b820@kili.mountain>
In-Reply-To: <95e36d70-7841-4e7d-960a-116b8595b820@kili.mountain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?sNEty3zSZHFdqjTxYLE5U6smv6EzPvdN58a+jFdQWZnWdMw5UG1V506LSKiH?=
 =?us-ascii?Q?9ZZIPs1sV3hGgNwI2tcKRkV61soYih24mEdTiWCgfXzU5ia12PDSVuvsbMrD?=
 =?us-ascii?Q?zM1StkGigva+7nUWl7JUJXryXDgQiHIw3kexx2Dn1w7HbInDXBQvNa/q6pxd?=
 =?us-ascii?Q?wrNo/GI5F5GN5oMEhk+LshfvAqphi3/tKaPLoAkX4rsOV6S1yOOdr4c+kv5C?=
 =?us-ascii?Q?6rw+s39JEWClo+/FA7mZXxBrew8F8X5q2ZFoVcadvvGlyGfSCFtOrlthIM0G?=
 =?us-ascii?Q?2vQ5LhrKyq60UTglKhrBA2d9F7GDbyfcLNznIbhvjkVOK349hADS2n0jEcOy?=
 =?us-ascii?Q?ciD/PM0SWU5li0XhXSXTVa6rZB+f0s9Jb+A51ryS6MAsGRmPG+xbSqQQVnVG?=
 =?us-ascii?Q?A15RRBZyFosnt7nJX6VpGQbfnhnF4AioWLrgt7zToS2S0VCdr5FxcX4VaKhs?=
 =?us-ascii?Q?hFc8l21bU0hda12GO5VG7x5Ke0rFZHED8PyL2SNCMzbGwnxqDHInsmdAn1wC?=
 =?us-ascii?Q?zYOSusPOJQ6cePm/IXdrz2P95Io1p+JONzqouov/1peeWIhyVDpCTMVDCUQX?=
 =?us-ascii?Q?R0rWqP6UdkQnrgRC5c3B4l4wnpAqrgSqKdIm279aMZiC6uQga8V6/JC87NaK?=
 =?us-ascii?Q?ke2qYZXPkvq8zKZM0D3RognRE3O6IlLqToBWJc4024nNEYw6qCuYuvHZu15Q?=
 =?us-ascii?Q?BCKPltsBDjJ95Gu1/Ef2GAJhi7UXngVd4dYxmyk1VBVoo6f6bbJVqWg+7fY2?=
 =?us-ascii?Q?ftJWIXth0WJiCJaItoWMnpTCbMd5Aw1GLZeVzWgylFYhqUTYJTQ0cf4qDljg?=
 =?us-ascii?Q?SL9Fzn0OjwVgpNK54jYHajQh4JPIE3B1LmBc30zuoEK8sUW6sP2Lfq56Pohc?=
 =?us-ascii?Q?AVTl5msCpoz9DVeOsGLvh4jDyz/G/l7j6mcRqmbmtw7E1HEnA5IrHbEkHkQQ?=
 =?us-ascii?Q?eMk1y0lTnr7uIsS82VMFQHcvUCbRE3NLcLLa+1Uo46fMDIuz0eGXYLEjr9bM?=
 =?us-ascii?Q?gF2Ja2l7dF6411ehGxMq8v0qbjfPv2DC7sdlaWHnJN0bitJ3VfH2c024T/8M?=
 =?us-ascii?Q?CDAFcxCp+v9CT2/yHM3fIt31iRpdZV+ryLCzr+IqTazjzQRb//XBK0KlxqA9?=
 =?us-ascii?Q?60R8SuPZYIGtntZqDE/eWB713MEPeCo/uSfftxeNxZF8f4oPVoQGIUN7XwbO?=
 =?us-ascii?Q?PhL7N9qMmu+XwcvS/wDan0OrsQ2T8iruvYT2s0p2SeZa7ZDxkNEjKpjC+eQS?=
 =?us-ascii?Q?crwodstTCbldMiHusHmL/a22z3emURsnY0kXCAlvaHC7d7f5js84q9lUOVlC?=
 =?us-ascii?Q?9iYwbuGXaHtq80UpQ74+vcHQMgvhnhTtMMHts6U+1HDaA6BjfVK0Y7YyIo7B?=
 =?us-ascii?Q?jNeJfmob7JUZbsFr/IyZnHvGzQjsq54f+bvdQW2wwMzfGVUHZNvZShb1tqef?=
 =?us-ascii?Q?DnmMeezYit+GX8tESck1lyuf06fAqWI1eZGT/JDQfNPDItEZUYD5ncZjF4SS?=
 =?us-ascii?Q?/H5oJFS4woUCwhAftcbDQUE93Js2DLGRB1gISclLdz91vp7JgnmN6KQBNN2q?=
 =?us-ascii?Q?5SE5F9k2fZo1vCUNWlP6TgGV+x+eg0oX7qFzAM6lcpcp9doCfo84BdQslp7w?=
 =?us-ascii?Q?3Q=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C6282723E0B15E4D83F1C9456B6220D7@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: zqbB+DBNs1nB1Va2V/3pDae9+xpNK/Pq288CmjMJGgVVOdXcaXPxJyJAIW0z5RkWeqXopmuTyb6ZH2M7T8ZKhBsnkBTSpnkBFxN6eDa68sQOAIlaUN8fWFGLPWznETjZrQ3jUkIe31E5anbZiLZ4bP1+RY3g97un/uQazj7WVRqqItdoOI6EGfI5bmrCHw2Ue7f/e/D2mI9PDZnTjXpYxgoBSIDp6tokewpfcasoQWJGx8KQBisjrVCwSchUdF9VjgoE69fGr8udyVmr+bF5GFOxjpYcyyIczWBra01Wm4ZnqTCE0Mt3Ln6IN4MOz5FB1hmrdNYw1ajEijTrq5LuPfCijUsnW8dxloqgUP5ZXzWMqSmsVSBnbbFmYr4BNJpOD69jIyAqd1XO9anxbWiNCJ9X6OZQy8ZQLpHhRay7UVY++lN9EbiEbjglw5s5ztCh5m4JFtYbDxpdAHJ1H0exbYX/WeopQazkN6qAR8S0D2/ebWQzOWLOSL19AJ7DsWQmclxv8jNzIq52wxfPpXhnJC5j9le/HnRbTFfP+N5qtY0btK2aNAiegk+S/JJxrZKBVh86VGX9V58wnly70b6N4I+tIWaTEBAYW3W9BS0F+wiQ14N6GWvA7Ua3DpgKXTKghVQGXQJSTRPnfi1FHDeFAQygcmFXfbWxssyjEiKzwoUq3H1zTt+DQFmqqgyKFYlokBIZY12vS60E0YqQfPsRZ1o+U+ImS7ZDu3RUZ7rbxWS8eSQc7Xloo1PANfdfqRMFZJbo6mR2hupHZclAS5WDa8cNpqeaGeHx5SuP9RS3yRlI4ZtK7aTk7AAJ3hpfztOU1xHvHOMjjRVS+SSrd0N2PiXRBgU7Z0lwlbLH/Z27VUxz9Vd5NfKSb6zHqfkFHCdaHGK9Zud6duk9IvETWlcotQ==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f2fef3d9-f1e2-4e77-3de0-08db4cb5db5e
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 May 2023 15:40:15.6060
 (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: bYcUgfwcfRrgIiQ9AEYvRqbcrTanEnk4U2M73kJ1wOh4nJSDFSSq41hwg4f4xiS63Ilh8F2QUMP/wIZNTrcAKA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB4122
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-04_10,2023-05-04_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=953
 phishscore=0 bulkscore=0 suspectscore=0 mlxscore=0 spamscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2305040128
X-Proofpoint-GUID: qklbZ2Ay6_KKfaNZ1JuTEMk3tmXbgPJB
X-Proofpoint-ORIG-GUID: qklbZ2Ay6_KKfaNZ1JuTEMk3tmXbgPJB
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 May 3, 2023, at 7:31 AM, Dan Carpenter <dan.carpenter@linaro.org> wrot=
e:
>=20
> I changed the CC list from netdev to linux-nfs.
>=20
> On Tue, Mar 07, 2023 at 02:37:07PM -0300, Thadeu Lima de Souza Cascardo w=
rote:
>> When sock_alloc_file fails to allocate a file, it will call sock_release=
.
>> __sys_socket_file should then not call sock_release again, otherwise the=
re
>> will be a double free.
>>=20
>=20
> It's just a one liner to make Smatch warn about these sorts of user
> after free bugs.
>=20
> { "sock_release", PARAM_FREED, 0, "$" },
>=20
> Smatch found a related bug in nfs.
>=20
> net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: 'newsock' was already fre=
ed.
> net/sunrpc/svcsock.c:1633 svc_create_socket() warn: 'sock' was already fr=
eed.
> net/sunrpc/svcsock.c:1555 svc_addsock() error: dereferencing freed memory=
 'so'
> net/sunrpc/svcsock.c:1633 svc_create_socket() warn: passing freed memory =
'sock'
> net/sunrpc/svcsock.c:938 svc_tcp_accept() warn: passing freed memory 'new=
sock'
>=20
> I guess the svc_setup_socket() function should free sock on every error
> path?  It seems kind of gnarly.

I'm looking at current upstream, and I don't see svc_setup_socket()
releasing @sock on any return path. What am I missing?

Does your tree have "SUNRPC: Ensure server-side sockets have a sock->file" =
?


--
Chuck Lever