Messages in this thread | | | Date | Thu, 9 Feb 2023 23:57:17 +0530 | Subject | Re: [PATCH] usb: gadget: u_serial: Add null pointer check in gserial_resume | From | Prashanth K <> |
| |
On 09-02-23 09:33 pm, Alan Stern wrote: > On Thu, Feb 09, 2023 at 09:13:37PM +0530, Prashanth K wrote: >> >> >> On 09-02-23 08:39 pm, Alan Stern wrote: >>> You should consider having _two_ spinlocks: One in the gs_port structure >>> (the way it is now) and a separate global lock. The first would be used >>> in situations where you know you have a valid pointer. The second would >>> be used in situations where you don't know if the pointer is non-NULL >>> or where you are changing the pointer's value. >> Lets say we replaced the existing spinlock in gserial_resume and >> gserial_disconnect with a new static spinlock, and kept the spinlocks in >> other functions unchanged. In that case, wouldn't it cause additional race >> conditions as we are using 2 different locks. > > Not race conditions, but possibilities for deadlock. > > Indeed, you would have to be very careful about avoiding deadlock > scenarios. In particular, you would have to ensure that the code never > tries to acquire the global spinlock while already holding one of the > per-port spinlocks. > > Alan Stern Hi Alan, instead of doing these and causing potential regressions, can we just have the null pointer check which i suggested in the beginning? The major concern was that port might become null after the null pointer check. We mark gser->ioport as null pointer in gserial_disconnect, and in gserial_resume we copy the gser->ioport to *port in the beginning.
struct gs_port *port = gser->ioport;
And hence it wont cause null pointer deref after the check as we don't de-reference anything from gser->ioport afterwards. We only use the local pointer *port afterwards.
Thanks, Prashanth K
| |