[PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Jeongjun Park]

In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
management"), the ata_cdl structure was added and the ata_cdl structure
memory was allocated with kzalloc(). Because of this, if CDL is not 
supported, dev->cdl is a NULL pointer, so additional work should never 
be done.

However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
dereferencing dev->cdl will result in a NULL pointer dereference.

Therefore, I think it is appropriate to check dev->flags in 
ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.

Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 drivers/ata/libata-scsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3328a6febc13..6f5527f12b0e 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
 	if (spg) {
 		switch (spg) {
 		case ALL_SUB_MPAGES:
-			break;
+			if (dev->flags & ATA_DFLAG_CDL)
+				break;
+			fallthrough;
 		case CDL_T2A_SUB_MPAGE:
 		case CDL_T2B_SUB_MPAGE:
 		case ATA_FEATURE_SUB_MPAGE:
--

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Damien Le Moal]

On 2024/09/21 14:41, Jeongjun Park wrote:
> In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
> management"), the ata_cdl structure was added and the ata_cdl structure
> memory was allocated with kzalloc(). Because of this, if CDL is not 
> supported, dev->cdl is a NULL pointer, so additional work should never 
> be done.
> 
> However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
> dereferencing dev->cdl will result in a NULL pointer dereference.
> 
> Therefore, I think it is appropriate to check dev->flags in 
> ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
> 
> Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  drivers/ata/libata-scsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3328a6febc13..6f5527f12b0e 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
>  	if (spg) {
>  		switch (spg) {
>  		case ALL_SUB_MPAGES:
> -			break;
> +			if (dev->flags & ATA_DFLAG_CDL)
> +				break;
> +			fallthrough;

I do not think this is correct at all. If the user request all sub mpages, we
need to give that list regardless of CDL support. What needs to be fixed is that
if CDL is NOT supported, we should not try to add the information for the T2A
and T2B sub pages. So the fix should be this:

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3328a6febc13..6ffa975746a6 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2256,10 +2256,15 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf)
 static unsigned int ata_msense_control_spgt2(struct ata_device *dev, u8 *buf,
                                             u8 spg)
 {
-       u8 *b, *cdl = dev->cdl->desc_log_buf, *desc;
+       u8 *b, *cdl, *desc;
        u32 policy;
        int i;

+       if (!(dev->flags & ATA_DFLAG_CDL) || !dev->cdl)
+               return 0;
+
+       cdl = dev->cdl->desc_log_buf;
+
        /*
         * Fill the subpage. The first four bytes of the T2A/T2B mode pages
         * are a header. The PAGE LENGTH field is the size of the page


>  		case CDL_T2A_SUB_MPAGE:
>  		case CDL_T2B_SUB_MPAGE:
>  		case ATA_FEATURE_SUB_MPAGE:
> --

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Jeongjun Park]

Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 2024/09/21 14:41, Jeongjun Park wrote:
> > In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
> > management"), the ata_cdl structure was added and the ata_cdl structure
> > memory was allocated with kzalloc(). Because of this, if CDL is not
> > supported, dev->cdl is a NULL pointer, so additional work should never
> > be done.
> >
> > However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
> > dereferencing dev->cdl will result in a NULL pointer dereference.
> >
> > Therefore, I think it is appropriate to check dev->flags in
> > ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
> >
> > Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> > Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> > Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > ---
> >  drivers/ata/libata-scsi.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index 3328a6febc13..6f5527f12b0e 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> > @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
> >       if (spg) {
> >               switch (spg) {
> >               case ALL_SUB_MPAGES:
> > -                     break;
> > +                     if (dev->flags & ATA_DFLAG_CDL)
> > +                             break;
> > +                     fallthrough;
>
> I do not think this is correct at all. If the user request all sub mpages, we
> need to give that list regardless of CDL support. What needs to be fixed is that
> if CDL is NOT supported, we should not try to add the information for the T2A
> and T2B sub pages. So the fix should be this:

Okay. But after looking into it further, I think it would be more appropriate to
also check the ATA_DFLAG_CDL_ENABLED flag when checking if CDL is
not supported. So it seems like it would be better to modify the condition as
below.

What do you think?

if (!(dev->flags & ATA_DFLAG_CDL
      dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
        return 0;

Regards,
Jeongjun Park

>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3328a6febc13..6ffa975746a6 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2256,10 +2256,15 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf)
>  static unsigned int ata_msense_control_spgt2(struct ata_device *dev, u8 *buf,
>                                              u8 spg)
>  {
> -       u8 *b, *cdl = dev->cdl->desc_log_buf, *desc;
> +       u8 *b, *cdl, *desc;
>         u32 policy;
>         int i;
>
> +       if (!(dev->flags & ATA_DFLAG_CDL) || !dev->cdl)
> +               return 0;
> +
> +       cdl = dev->cdl->desc_log_buf;
> +
>         /*
>          * Fill the subpage. The first four bytes of the T2A/T2B mode pages
>          * are a header. The PAGE LENGTH field is the size of the page
>
>
> >               case CDL_T2A_SUB_MPAGE:
> >               case CDL_T2B_SUB_MPAGE:
> >               case ATA_FEATURE_SUB_MPAGE:
> > --
>
> --
> Damien Le Moal
> Western Digital Research
>

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Jeongjun Park]

Jeongjun Park <aha310510@gmail.com> wrote:
>
> Damien Le Moal <dlemoal@kernel.org> wrote:
> >
> > On 2024/09/21 14:41, Jeongjun Park wrote:
> > > In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
> > > management"), the ata_cdl structure was added and the ata_cdl structure
> > > memory was allocated with kzalloc(). Because of this, if CDL is not
> > > supported, dev->cdl is a NULL pointer, so additional work should never
> > > be done.
> > >
> > > However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
> > > dereferencing dev->cdl will result in a NULL pointer dereference.
> > >
> > > Therefore, I think it is appropriate to check dev->flags in
> > > ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
> > >
> > > Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> > > Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> > > Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
> > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > ---
> > >  drivers/ata/libata-scsi.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > > index 3328a6febc13..6f5527f12b0e 100644
> > > --- a/drivers/ata/libata-scsi.c
> > > +++ b/drivers/ata/libata-scsi.c
> > > @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
> > >       if (spg) {
> > >               switch (spg) {
> > >               case ALL_SUB_MPAGES:
> > > -                     break;
> > > +                     if (dev->flags & ATA_DFLAG_CDL)
> > > +                             break;
> > > +                     fallthrough;
> >
> > I do not think this is correct at all. If the user request all sub mpages, we
> > need to give that list regardless of CDL support. What needs to be fixed is that
> > if CDL is NOT supported, we should not try to add the information for the T2A
> > and T2B sub pages. So the fix should be this:
>
> Okay. But after looking into it further, I think it would be more appropriate to
> also check the ATA_DFLAG_CDL_ENABLED flag when checking if CDL is
> not supported. So it seems like it would be better to modify the condition as
> below.
>
> What do you think?
>
> if (!(dev->flags & ATA_DFLAG_CDL
>       dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
>         return 0;

if (!(dev->flags & ATA_DFLAG_CDL &&
      dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
        return 0;

>
> Regards,
> Jeongjun Park
>
> >
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index 3328a6febc13..6ffa975746a6 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> > @@ -2256,10 +2256,15 @@ static inline u16 ata_xlat_cdl_limit(u8 *buf)
> >  static unsigned int ata_msense_control_spgt2(struct ata_device *dev, u8 *buf,
> >                                              u8 spg)
> >  {
> > -       u8 *b, *cdl = dev->cdl->desc_log_buf, *desc;
> > +       u8 *b, *cdl, *desc;
> >         u32 policy;
> >         int i;
> >
> > +       if (!(dev->flags & ATA_DFLAG_CDL) || !dev->cdl)
> > +               return 0;
> > +
> > +       cdl = dev->cdl->desc_log_buf;
> > +
> >         /*
> >          * Fill the subpage. The first four bytes of the T2A/T2B mode pages
> >          * are a header. The PAGE LENGTH field is the size of the page
> >
> >
> > >               case CDL_T2A_SUB_MPAGE:
> > >               case CDL_T2B_SUB_MPAGE:
> > >               case ATA_FEATURE_SUB_MPAGE:
> > > --
> >
> > --
> > Damien Le Moal
> > Western Digital Research
> >

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Damien Le Moal]

On 2024/09/23 13:15, Jeongjun Park wrote:
> Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> On 2024/09/21 14:41, Jeongjun Park wrote:
>>> In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
>>> management"), the ata_cdl structure was added and the ata_cdl structure
>>> memory was allocated with kzalloc(). Because of this, if CDL is not
>>> supported, dev->cdl is a NULL pointer, so additional work should never
>>> be done.
>>>
>>> However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
>>> dereferencing dev->cdl will result in a NULL pointer dereference.
>>>
>>> Therefore, I think it is appropriate to check dev->flags in
>>> ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
>>>
>>> Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
>>> Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
>>> Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
>>> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
>>> ---
>>>  drivers/ata/libata-scsi.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>> index 3328a6febc13..6f5527f12b0e 100644
>>> --- a/drivers/ata/libata-scsi.c
>>> +++ b/drivers/ata/libata-scsi.c
>>> @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
>>>       if (spg) {
>>>               switch (spg) {
>>>               case ALL_SUB_MPAGES:
>>> -                     break;
>>> +                     if (dev->flags & ATA_DFLAG_CDL)
>>> +                             break;
>>> +                     fallthrough;
>>
>> I do not think this is correct at all. If the user request all sub mpages, we
>> need to give that list regardless of CDL support. What needs to be fixed is that
>> if CDL is NOT supported, we should not try to add the information for the T2A
>> and T2B sub pages. So the fix should be this:
> 
> Okay. But after looking into it further, I think it would be more appropriate to
> also check the ATA_DFLAG_CDL_ENABLED flag when checking if CDL is
> not supported. So it seems like it would be better to modify the condition as
> below.
> 
> What do you think?
> 
> if (!(dev->flags & ATA_DFLAG_CDL
>       dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
>         return 0;

No, that would be wrong. The mode sense is to report if CDL is *supported*, not
if it is enabled or not. So we always must report the T2A and T2B pages for SATA
drives that support CDL, even if the CDL feature is disabled.

The flag ATA_DFLAG_CDL_ENABLED is not checked in ata_scsiop_mode_sense() for
this reason. Adding that check in ata_msense_control_spgt2() would be wrong.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Jeongjun Park]

Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 2024/09/23 13:15, Jeongjun Park wrote:
> > Damien Le Moal <dlemoal@kernel.org> wrote:
> >>
> >> On 2024/09/21 14:41, Jeongjun Park wrote:
> >>> In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
> >>> management"), the ata_cdl structure was added and the ata_cdl structure
> >>> memory was allocated with kzalloc(). Because of this, if CDL is not
> >>> supported, dev->cdl is a NULL pointer, so additional work should never
> >>> be done.
> >>>
> >>> However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
> >>> dereferencing dev->cdl will result in a NULL pointer dereference.
> >>>
> >>> Therefore, I think it is appropriate to check dev->flags in
> >>> ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
> >>>
> >>> Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> >>> Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
> >>> Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
> >>> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> >>> ---
> >>>  drivers/ata/libata-scsi.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> >>> index 3328a6febc13..6f5527f12b0e 100644
> >>> --- a/drivers/ata/libata-scsi.c
> >>> +++ b/drivers/ata/libata-scsi.c
> >>> @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
> >>>       if (spg) {
> >>>               switch (spg) {
> >>>               case ALL_SUB_MPAGES:
> >>> -                     break;
> >>> +                     if (dev->flags & ATA_DFLAG_CDL)
> >>> +                             break;
> >>> +                     fallthrough;
> >>
> >> I do not think this is correct at all. If the user request all sub mpages, we
> >> need to give that list regardless of CDL support. What needs to be fixed is that
> >> if CDL is NOT supported, we should not try to add the information for the T2A
> >> and T2B sub pages. So the fix should be this:
> >
> > Okay. But after looking into it further, I think it would be more appropriate to
> > also check the ATA_DFLAG_CDL_ENABLED flag when checking if CDL is
> > not supported. So it seems like it would be better to modify the condition as
> > below.
> >
> > What do you think?
> >
> > if (!(dev->flags & ATA_DFLAG_CDL
> >       dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
> >         return 0;
>
> No, that would be wrong. The mode sense is to report if CDL is *supported*, not
> if it is enabled or not. So we always must report the T2A and T2B pages for SATA
> drives that support CDL, even if the CDL feature is disabled.
>
> The flag ATA_DFLAG_CDL_ENABLED is not checked in ata_scsiop_mode_sense() for
> this reason. Adding that check in ata_msense_control_spgt2() would be wrong.
>

Thanks for your reply! I will write a v2 patch to meet the
requirements you provided
and send it to you.

Regards,
Jeongjun Park

>
> --
> Damien Le Moal
> Western Digital Research

Re: [PATCH] ata: libata: fix ALL_SUB_MPAGES not to be performed when CDL is not supported

[Damien Le Moal]

On 2024/09/23 15:22, Jeongjun Park wrote:
> Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> On 2024/09/23 13:15, Jeongjun Park wrote:
>>> Damien Le Moal <dlemoal@kernel.org> wrote:
>>>>
>>>> On 2024/09/21 14:41, Jeongjun Park wrote:
>>>>> In the previous commit 602bcf212637 ("ata: libata: Improve CDL resource
>>>>> management"), the ata_cdl structure was added and the ata_cdl structure
>>>>> memory was allocated with kzalloc(). Because of this, if CDL is not
>>>>> supported, dev->cdl is a NULL pointer, so additional work should never
>>>>> be done.
>>>>>
>>>>> However, even if CDL is not supported now, if spg is ALL_SUB_MPAGES,
>>>>> dereferencing dev->cdl will result in a NULL pointer dereference.
>>>>>
>>>>> Therefore, I think it is appropriate to check dev->flags in
>>>>> ata_scsiop_mode_sense() if spg is ALL_SUB_MPAGES to see if CDL is supported.
>>>>>
>>>>> Reported-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
>>>>> Tested-by: syzbot+37757dc11ee77ef850bb@syzkaller.appspotmail.com
>>>>> Fixes: 602bcf212637 ("ata: libata: Improve CDL resource management")
>>>>> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
>>>>> ---
>>>>>  drivers/ata/libata-scsi.c | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>>>>> index 3328a6febc13..6f5527f12b0e 100644
>>>>> --- a/drivers/ata/libata-scsi.c
>>>>> +++ b/drivers/ata/libata-scsi.c
>>>>> @@ -2442,7 +2442,9 @@ static unsigned int ata_scsiop_mode_sense(struct ata_scsi_args *args, u8 *rbuf)
>>>>>       if (spg) {
>>>>>               switch (spg) {
>>>>>               case ALL_SUB_MPAGES:
>>>>> -                     break;
>>>>> +                     if (dev->flags & ATA_DFLAG_CDL)
>>>>> +                             break;
>>>>> +                     fallthrough;
>>>>
>>>> I do not think this is correct at all. If the user request all sub mpages, we
>>>> need to give that list regardless of CDL support. What needs to be fixed is that
>>>> if CDL is NOT supported, we should not try to add the information for the T2A
>>>> and T2B sub pages. So the fix should be this:
>>>
>>> Okay. But after looking into it further, I think it would be more appropriate to
>>> also check the ATA_DFLAG_CDL_ENABLED flag when checking if CDL is
>>> not supported. So it seems like it would be better to modify the condition as
>>> below.
>>>
>>> What do you think?
>>>
>>> if (!(dev->flags & ATA_DFLAG_CDL
>>>       dev->flags & ATA_DFLAG_CDL_ENABLED) || !dev->cdl)
>>>         return 0;
>>
>> No, that would be wrong. The mode sense is to report if CDL is *supported*, not
>> if it is enabled or not. So we always must report the T2A and T2B pages for SATA
>> drives that support CDL, even if the CDL feature is disabled.
>>
>> The flag ATA_DFLAG_CDL_ENABLED is not checked in ata_scsiop_mode_sense() for
>> this reason. Adding that check in ata_msense_control_spgt2() would be wrong.
>>
> 
> Thanks for your reply! I will write a v2 patch to meet the
> requirements you provided
> and send it to you.

No need. I have the fix patch already. And while writing it, I also noticed that
ata_msense_control() is wrong (the ALL_SUB_MPAGES case adds the T2A page twice
instead of adding the T2A page and then the T2B page. So I have another patch to
fix that.

Will be posting soon.

> 
> Regards,
> Jeongjun Park
> 
>>
>> --
>> Damien Le Moal
>> Western Digital Research


-- 
Damien Le Moal
Western Digital Research