Re: [PATCH v3 5/7] drivers: greybus: Add software SVC implementation

[Ayush Singh <ayush@beagleboard.org>]

sashiko-bot@kernel.org writes:

> [ ... ]
>
>> +    fn send_request<P: AsBytes>(&self, hd: &hd::Device, ty: u8, payload: P) -> Result<()> {
>> +        let msg = Msg::request(self.new_operation_id()?, ty, payload);
>> +        hd.data_rcvd(protocols::GB_SVC_CPORT_ID, msg.as_bytes());
>> +        Ok(())
>> +    }
>
> [Severity: Medium]
> Is there a mechanism to prevent operation_id leaks if the core drops SVC
> requests?
>
> When gb_softsvc sends a request via hd.data_rcvd(), the greybus core might
> drop the request early (for instance, if the GFP_ATOMIC memory allocation
> fails in gb_operation_create_incoming() or gb_operation_get_active() fails
> in gb_connection_recv_request() in drivers/greybus/operation.c).
>
> If the core silently drops the message without calling
> gb_operation_request_handle() or sending a response back to the SVC,
> could the svc_operation_id IdPool leak IDs until find_unused_id() fails
> with EOVERFLOW, preventing the soft SVC from sending further requests?

Well, if this could somehow happen, that would be a bug in greybus core
and should be fixed. I will look into it, but I feel like a kernel module
should be able to trust the kernel itself.

Best Regards,
Ayush Singh

-- 
BeagleBoard.org Foundation
https://www.beagleboard.org