Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b5a845e9-1fa0-ea36-98c4-b5da989c44c6@oracle.com>
Date:   Thu, 1 Jun 2023 11:33:09 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.2
Subject: Re: [syzbot] [kvm?] [net?] [virt?] general protection fault in
 vhost_work_queue
To:     Stefano Garzarella <sgarzare@redhat.com>
Cc:     "Michael S. Tsirkin" <mst@redhat.com>,
        syzbot <syzbot+d0d442c22fa8db45ff0e@syzkaller.appspotmail.com>,
        jasowang@redhat.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        syzkaller-bugs@googlegroups.com,
        virtualization@lists.linux-foundation.org, stefanha@redhat.com
References: <0000000000001777f605fce42c5f@google.com>
 <20230530072310-mutt-send-email-mst@kernel.org>
 <CAGxU2F7O7ef3mdvNXtiC0VtWiS2DMnoiGwSR=Z6SWbzqcrBF-g@mail.gmail.com>
 <CAGxU2F7HK5KRggiY7xnKHeXFRXJmqcKbjf3JnXC3mbmn9xqRtw@mail.gmail.com>
 <e4589879-1139-22cc-854f-fed22cc18693@oracle.com>
 <6p7pi6mf3db3gp3xqarap4uzrgwlzqiz7wgg5kn2ep7hvrw5pg@wxowhbw4e7w7>
 <035e3423-c003-3de9-0805-2091b9efb45d@oracle.com>
 <CAGxU2F5oTLY_weLixRKMQVqmjpDG_09yL6tS2rF8mwJ7K+xP0Q@mail.gmail.com>
 <43f67549-fe4d-e3ca-fbb0-33bea6e2b534@oracle.com>
 <bbe697b6-dd9e-5a8d-21c5-315ab59f0456@oracle.com>
 <7vk2uizpmf4fi54tmmopnbwwb7fs2xg6vae6ynrcvs26hjmshb@hpjzu4jfj35i>
Content-Language: en-US
From:   Mike Christie <michael.christie@oracle.com>
In-Reply-To: <7vk2uizpmf4fi54tmmopnbwwb7fs2xg6vae6ynrcvs26hjmshb@hpjzu4jfj35i>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SkVkNXlZWWh3VTg4Z2hrM0xsNGRqaGdnMFN3ZVdKZ3h1Ni83a1lNeUlCOGpO?=
 =?utf-8?B?bHUzZDF6bWFVTDlWU0N3ZEs5M3VRVE1zZ1E2MFVwR3RpcnpNNVpBZmhaQytz?=
 =?utf-8?B?WCszZ1RTV24wenQxd0hXUGZiZE0xMHZoZzh4TU5iZ3Flb3BDTVlOU2Nrellk?=
 =?utf-8?B?QTVvM0tZa2tPZGVGTGdKOGNOdjNScktrWTY2Y1BycFhhK1BYSlh4aFQ0OEFq?=
 =?utf-8?B?N2tLOFJYK3A2bW5OWGFLOE1pekM2Lzc4SFRPaVlBWkRqcTVRMVZGaTY3Y2NF?=
 =?utf-8?B?aWVEbEN5M2tSb3ZialdaaHJ6ZzhyWXJGdlQxVkJMd0hrYWJRTElhdjhNTHdT?=
 =?utf-8?B?NTJRVGdrLy9EU3dtTk02TWhma3g5QUN6ZnFpY3FjellJUHpHbk1DRDFGODZ5?=
 =?utf-8?B?RHEzNldKWU9WTHk4K1gyZU1UVkpybC8zN0NCWEFmaFh4Rk0xdGQ2TFdZQ2Fa?=
 =?utf-8?B?U2RsQUJFQUNvRm5STmNMM0Z4bU52bG5OdUlWY2tCcGJyMm9GUW44SEhEQzVx?=
 =?utf-8?B?RnRkVnljdHVKbVo4ZmdkWWpyZ0lzbUc2dXZLWnBIS25ReWZFN1hJelNFaElJ?=
 =?utf-8?B?RjdYN3dRZnUwY1BmdVduczI0V002R1AzSlZjb1VOTVpMc3RlYVUzUXV0bm4x?=
 =?utf-8?B?OUR1OTU5ZUZuSWE4WVYxU0FIV3lsV2E1dDVUanhKZGZadFZpNkc0VjhEVkVB?=
 =?utf-8?B?VnV4V2FBdzNERTdjdXlkK2RKTEdzSWVBZ3h0U3pqallCWU5GWDJvVXllMmIz?=
 =?utf-8?B?R3gwdCtjZUdNT3Z5OUxHbUFhSFhkNlRUQ25ySTNrT0NTb2RveGRqdVB6TUsr?=
 =?utf-8?B?aVpBdFc4L1FxWDFJL1dPUm1zM216NGZJcmpsT0FtZU0yN0JROWc3eWN5WW8z?=
 =?utf-8?B?U0crOW1SN0laS0ZiMWVGeGJueHlWOXp4ODhycmFsT05BMjV2UDJoWHVqOGhB?=
 =?utf-8?B?eFVzTUJDT1M1QmhEcFJRM0tWVTRlcGRsR2hocHN0RzZ5MnQxOU9GbmZiMXJ5?=
 =?utf-8?B?d3lvODVRd3V5a2grcFVlYlMwcjU1QWNIUEUyYVg1c3FXcEJ6Yy80L1hNS1Rq?=
 =?utf-8?B?bGFRQWtVOGhST0dmYTVORERQTDVGYzlGZVNBeG1UR3JTTEZGZGhrc1VPWng5?=
 =?utf-8?B?bWNydEdXekVzSzNnbmNwVUIySjBPM1pmNzBVYlkvT3ZSRFJBeFZxODU0SERs?=
 =?utf-8?B?SjVzZjlYWUh1ZnVKbDR6UXNkZ1JyV2haVkMzQ0FQY1EweGQ1TGhrMFRjWHZl?=
 =?utf-8?B?UzdJMG1ubkVaeTRSWFVEUzZyZzZwTEhCUDZrZ3VSenpWMGxJaE9jWjlWVTlS?=
 =?utf-8?B?YkRBMjFBczRJN09OYnVSVmFBb1R5MDlGakZraGQyK25KRVhaU3NnblU1VkJz?=
 =?utf-8?B?WTZLZERVZGNYa3Zyd0RLS2g3ZXNtbVp5b3JDSjNDK1JEUVlGd1p4c0NiMitY?=
 =?utf-8?B?UGJ6Smw5WG9CUmR5S0ZBQjdoV2UvNUQ5V2l5RjFsQVFLcmN2ZGY3aU5pekNN?=
 =?utf-8?B?S1lqNm9hR0pqUU9rL2tnOHNYSmlhbTl5dE1sNnVsOCtmT2U5VWhFMS9lYlJr?=
 =?utf-8?B?c2xGUGxJdjhrQ3lGOGdwSnB3Ym5SN2lZaVNWZWQ2eUY0ZlVuVmg1RE9OSXZR?=
 =?utf-8?B?R01DSlV1NVoyUWk1cThCejBibW04Tlk4V2tyRHlBNlh5djU5TkNhdzdsc08v?=
 =?utf-8?B?OWg5bHZVVENwU2NRa0hxLzdBNW5PNnFEK3AvMlJ6emVrcS80aUVwcGJXandC?=
 =?utf-8?B?cHVsV3BpeWlnU2xHdUFsVFFqc2lML2RiUzJkcVNUS0RnUmRUZGoyQkdDTVQy?=
 =?utf-8?B?OXpKR2lUeGVROHk0NGp1cWhVeml3NVIrWjZZRHdXRGxjeU1UdFlreCtYYlRJ?=
 =?utf-8?B?R2RKMFpPd2hIVTRaaEgwWlM0ZFR5MmljV0FlaWJFNlRtWkFYWWtBOXJ3OENL?=
 =?utf-8?B?R2M2RzBlS0hzVnNtWXlvcGd3MmZsNVJqYWlWYkM4ejRuTE82RzRaUEswVU5W?=
 =?utf-8?B?ZXJRVExXZEpyOWRxZTY3aDlCREsvMCtHTVY2bFRMVmxvTER3YVFiWExKSEFK?=
 =?utf-8?B?eWtid0NURVQ5Vm5RMjNXSTRmeldoU0ZBOWlBUmlmT0J3elNUTHErOFNoNGoz?=
 =?utf-8?B?ZnpKb0FIMkU2TmNoSUlDTW9VTEp3NE1LY0xtU2c5WFR5bGR4TUhidm10c3FV?=
 =?utf-8?B?bkE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?NC9ZTFQ4WkZUcyt5aUNHcGYrZUNVeWl1NW0yaVdua3JTa2x2RkIwWktPZFA0?=
 =?utf-8?B?REY3UlFUczJwZmZKY3VzZm10Z0pvZEhZK0EwTkpkS2FBczdSV0FzQVdiQUZ1?=
 =?utf-8?B?WWNxNWZ2cjRsR3k1YVlKMWJpWnhrR2JINGQxOXlvNTZvblB3QVhFWW8vNXBN?=
 =?utf-8?B?NCtnNGhRem9uTVhWSElrN0k5S3FZZzdIWXhxQ0ttS1NCVnN1MXduTVZGdXp5?=
 =?utf-8?B?LytCaTN2dWZxd3o4YUpDMmVCZVlJbDFDWGZPdjZCQ2JuWE9MREpidTdmMi81?=
 =?utf-8?B?cmJoaGRidGdRaWJ2VXVYUnpRTGo0aTA0VnZ6aVhGYnJ2OGVpMC95K2M5UWxO?=
 =?utf-8?B?d3R3NkVnUUJxVUJ5SnNtTVNBa3NFS0UrdER1ZWZQdXdZVk9WZXBFN24wYldS?=
 =?utf-8?B?VlJNZiszKzRWTXE4M1BLMjdvMGdFRmhodzJNUG9nbGJKMnhYeTBuVVBaUkEy?=
 =?utf-8?B?VzY4bE5OYWNhbGpMRmpHNUFTTGcrMnVBcFpLVGpxeVM5Z1ZqN1E1SE5MUUk0?=
 =?utf-8?B?aSthTmNwc0xwYnpZc0xzWXAreU0vYjR2T2lWbmpCZFU4eHBaZEpZOUJqWFZz?=
 =?utf-8?B?dHFkTFpwakhzNlNtOS96M2VDT2UzakRzbXhKNy9xN1hOVGRaTVlCZURWbjQ4?=
 =?utf-8?B?NDRJd3VMRmVuaHd1SkNuMVBqZlJTQnl2NU1ENFZOUGY2MEFhZU5JTGJ3THhl?=
 =?utf-8?B?NGVPMUhQcS92MWtFYXNOOFNHR2xHNkI4UWJndCtkbU1jRTlSR0xJT3E0Mzlx?=
 =?utf-8?B?ZWVUd1NBOTZMRytTYjh3Vi9RUkhnRVdKa0xYbms0bmFHREhNdEpxdHRoOERH?=
 =?utf-8?B?YjBVV1VMZ3p4SVNReFVyNGNEblBqNytPekcrYXMxckx4Nk1RQk51cHdQZlZ1?=
 =?utf-8?B?SkR4SEhhN2cvU00vQjlVQXdDa0R0RzNTcDB0Y1VVVTY1czI0UmdVMlg4Z1Ar?=
 =?utf-8?B?MkNvZlJxVW9oMlVsUS9iZGpQTkcxNEUvZ2NnTHdlU2JxSU53SUlMdytuZkFi?=
 =?utf-8?B?VE5zT3hOVHNqTHRaeHFhUkh3bkNiV1VMTm1GbkJLM01wL1BGeXJ4QU9wQjhF?=
 =?utf-8?B?VG5DT2cwdTFobXdYMHFMWm5vS002TTZTS1FhUDVEVXA1aDRpWWIvNXNlK1ha?=
 =?utf-8?B?enkrTTMvV0s2NStyb2dzcXRNQ2xTTHYwby9WS21Jb0JrNk5rQjB4ZzFmaFpV?=
 =?utf-8?B?MmtyQXhNSFh4Wmd5M21jL044V2RDVWs2VFhuNTZxY3c4Y2Myb3JCRG5DZmpQ?=
 =?utf-8?B?S1p1dUVBQk5LdDZXY3lUeERmM3V6U0tKLzJnUXMreEUwdUdZUExHY3lGa05i?=
 =?utf-8?Q?qrB1jKUqVrRYc=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 45c90095-a072-41f4-a938-08db62bde3b0
X-MS-Exchange-CrossTenant-AuthSource: CY8PR10MB7243.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Jun 2023 16:33:11.2551
 (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: vyV/1I0GbyPJtDwvI2I1LaTjnSlZvuycNqFZTQHWWtFBBfSVhsubWFFPn7QadUVFz7wXBH6+amPN1uQNo1hBiWL53otWEei4jSDmitw4AHI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB6725
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-01_08,2023-05-31_03,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 adultscore=0 spamscore=0
 phishscore=0 malwarescore=0 mlxlogscore=770 mlxscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2304280000
 definitions=main-2306010144
X-Proofpoint-GUID: yHHNFO-IelSXUJKehf1SO_Cg8l3LWYB_
X-Proofpoint-ORIG-GUID: yHHNFO-IelSXUJKehf1SO_Cg8l3LWYB_
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 6/1/23 2:47 AM, Stefano Garzarella wrote:
>>
>> static void vhost_worker_free(struct vhost_dev *dev)
>> {
>> -    struct vhost_worker *worker = dev->worker;
>> +    struct vhost_task *vtsk = READ_ONCE(dev->worker.vtsk);
>>
>> -    if (!worker)
>> +    if (!vtsk)
>>         return;
>>
>> -    dev->worker = NULL;
>> -    WARN_ON(!llist_empty(&worker->work_list));
>> -    vhost_task_stop(worker->vtsk);
>> -    kfree(worker);
>> +    vhost_task_stop(vtsk);
>> +    WARN_ON(!llist_empty(&dev->worker.work_list));
>> +    WRITE_ONCE(dev->worker.vtsk, NULL);
> 
> The patch LGTM, I just wonder if we should set dev->worker to zero here,

We might want to just set kcov_handle to zero for now.

In 6.3 and older, I think we could do:

1. vhost_dev_set_owner could successfully set dev->worker.
2. vhost_transport_send_pkt runs vhost_work_queue and sees worker
is set and adds the vhost_work to the work_list.
3. vhost_dev_set_owner fails in vhost_attach_cgroups, so we stop
the worker before the work can be run and set worker to NULL.
4. We clear kcov_handle and return.

We leave the work on the work_list.

5. Userspace can then retry vhost_dev_set_owner. If that works, then the
work gets executed ok eventually.

OR

Userspace can just close the device. vhost_vsock_dev_release would
eventually call vhost_dev_cleanup (vhost_dev_flush won't see a worker
so will just return), and that will hit the WARN_ON but we would
proceed ok.

If I do a memset of the worker, then if userspace were to retry
VHOST_SET_OWNER, we would lose the queued work since the work_list would
get zero'd. I think it's unlikely this ever happens, but you know best
so let me know if this a real issue.