On Sun, Apr 03, 2016 at 02:25:05PM -0400, Alan Stern wrote:
> On Sun, 3 Apr 2016, Navin P.S wrote:
>
> > On Sat, Apr 2, 2016 at 8:00 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > > On Sat, 2 Apr 2016, Navin P.S wrote:
> > >>regs->hostpc[(wIndex & 0xff) - 1];
> > >
> > > You're asking the question backwards. wIndex is allowed to be 0
> > > because the USB spec says so. You can't argue with that.
> > >
> > > You should be asking why we initialize status_reg and hostpc_reg as
> > > above.
> > >.
> >
> > Can i initialize them to NULL and only use them if wIndex is not zero
> > and wIndex <= ports.
>
> You can't use a NULL pointer! You have to set it to a non-NULL value
> before you can dereference it.
>
> > I assign them goto error case statement.
>
> I don't understand that sentence.
>
> > That would be a cleaner solution.
>
> It would not be cleaner than leaving the code the way it is.
I was proposing something like the attached.
When you have an oppurtunity to fix or be complaint let us use that so.
I finally leave this here for you to accept or reject.
>
> > >> This is only valid when we have regs->port_status and regs->hostpc and
> > >> 1 more into the actual array but gcc catches this.
> > >
> > > No, it's always valid. The C spec might not agree with me, but the
> > > kernel doesn't use standard C. It uses gcc, which is different from
> > > the standard in quite a few ways.
> > >
> >
> >
> > If you had told me the size of port_status and hostpc is 65536
> > atleast i wouldn't have a problem
>
> I don't see why you have a problem anyway. There's nothing wrong with
> assigning a nonsense value to a pointer, as long as you don't try to
> use it. For example, your compiler might not like this program, but
> the program won't cause an error when you run it:
>
> int a[5];
>
> int main()
> {
> int *x = &a[-1];
>
> return 0;
> }
>
I understand you are initializing a variable x with some value like you
do int x=5 or int x=5-1; I guess the C standard doesn't allow that
(like you said wIndex can be 0 as per the EHCI spec and specs cannot
be argued with).
gcc devels wouldn't even accept a patch because the C standard says
it is undefined behaviour. Maybe due to trap representation . Again
gcc doesn't catch 2nd level access. So even gcc/ubsan is at fault here.
int main()
{
/*This program returns 0 on gcc -fsanitize=undefined tt.c */
int arr[4]={1,2,3,4};
int *ptr=arr;
ptr--;
if(*ptr==ptr[0]) return 0;// should have aborted
return 1;
}
Clang catches the above but when you change it to
if(ptr ==&ptr[0]) it passes instead of aborting if it were
complaint to the spec.
Maybe to speed up things they don't actually track boundaries of
stack variables. Maybe like when you have 2 arrays what happens
if one pointer subtracts or adds and offset that goes exactly
into the address that second array lies. So we have to track
each pointer with valid address and you cannot assign anything
beyond that or use aliases to do that.
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index ffc9029..8e7e4a7 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -872,9 +872,8 @@ int ehci_hub_control(
) {
struct ehci_hcd *ehci = hcd_to_ehci (hcd);
int ports = HCS_N_PORTS (ehci->hcs_params);
- u32 __iomem *status_reg = &ehci->regs->port_status[
- (wIndex & 0xff) - 1];
- u32 __iomem *hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
+ u32 __iomem *status_reg = NULL;
+ u32 __iomem *hostpc_reg = NULL;
u32 temp, temp1, status;
unsigned long flags;
int retval = 0;
@@ -902,6 +901,9 @@ int ehci_hub_control(
case ClearPortFeature:
if (!wIndex || wIndex > ports)
goto error;
+
+ status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
+ hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
wIndex--;
temp = ehci_readl(ehci, status_reg);
temp &= ~PORT_RWC_BITS;
@@ -990,6 +992,8 @@ int ehci_hub_control(
case GetPortStatus:
if (!wIndex || wIndex > ports)
goto error;
+ status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
+ hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
wIndex--;
status = 0;
temp = ehci_readl(ehci, status_reg);
@@ -1159,6 +1163,8 @@ int ehci_hub_control(
}
if (!wIndex || wIndex > ports)
goto error;
+ status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
+ hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
wIndex--;
temp = ehci_readl(ehci, status_reg);
if (temp & PORT_OWNER)
